| Message ID | cover.1714982035.git.antony.antony@secunet.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fix icmp error source address over xfrm tunnel | expand |
2024-05-06, 09:58:26 +0200, Antony Antony wrote: > Hi, > This fix, originally intended for XFRM/IPsec, has been recommended by > Steffen Klassert to submit to the net tree. > > The patch addresses a minor issue related to the IPv4 source address of > ICMP error messages, which originated from an old 2011 commit: > > 415b3334a21a ("icmp: Fix regression in nexthop resolution during replies.") > > The omission of a "Fixes" tag in the following commit is deliberate > to prevent potential test failures and subsequent regression issues > that may arise from backporting this patch all stable kerenels. What kind of regression do you expect? If there's a risk of regression, I'm not sure net-next is that much "better" than net or stable. If a user complains about the new behavior breaking their setup, my understanding is that you would likely have to revert the patch anyway, or at least add some way to toggle the behavior.
Hi Sabrina, On Mon, May 06, 2024 at 03:36:15PM +0200, Sabrina Dubroca via Devel wrote: > 2024-05-06, 09:58:26 +0200, Antony Antony wrote: > > Hi, > > This fix, originally intended for XFRM/IPsec, has been recommended by > > Steffen Klassert to submit to the net tree. > > > > The patch addresses a minor issue related to the IPv4 source address of > > ICMP error messages, which originated from an old 2011 commit: > > > > 415b3334a21a ("icmp: Fix regression in nexthop resolution during replies.") > > > > The omission of a "Fixes" tag in the following commit is deliberate > > to prevent potential test failures and subsequent regression issues > > that may arise from backporting this patch all stable kerenels. > > What kind of regression do you expect? If there's a risk of For example, an old testing scripts with hardcoded source IP address assume that the "Unreachable response" will have the previous behavior. Such testing script may trigger regression when this patch is backported. Consequently, there may be discussions on whether this patch has broken the 10-year-old test scripts, which may be hard to fix. > regression, I'm not sure net-next is that much "better" than net or > stable. If a user complains about the new behavior breaking their > setup, my understanding is that you would likely have to revert the > patch anyway, or at least add some way to toggle the behavior. My hope is that if this patch is applied to net-next without a "Fixes" tag, users would fix their testing scripts properly. Additionally, another piece of the puzzle for a complete fix is "forwarding of ICMP Error messages" patch that is in the kerenl 6.8, which is new feature and applied via ipsec-next. -antony
2024-05-06, 17:57:23 +0200, Antony Antony wrote: > Hi Sabrina, > > On Mon, May 06, 2024 at 03:36:15PM +0200, Sabrina Dubroca via Devel wrote: > > 2024-05-06, 09:58:26 +0200, Antony Antony wrote: > > > Hi, > > > This fix, originally intended for XFRM/IPsec, has been recommended by > > > Steffen Klassert to submit to the net tree. > > > > > > The patch addresses a minor issue related to the IPv4 source address of > > > ICMP error messages, which originated from an old 2011 commit: > > > > > > 415b3334a21a ("icmp: Fix regression in nexthop resolution during replies.") > > > > > > The omission of a "Fixes" tag in the following commit is deliberate > > > to prevent potential test failures and subsequent regression issues > > > that may arise from backporting this patch all stable kerenels. > > > > What kind of regression do you expect? If there's a risk of > > For example, an old testing scripts with hardcoded source IP address assume > that the "Unreachable response" will have the previous behavior. Such > testing script may trigger regression when this patch is backported. > Consequently, there may be discussions on whether this patch has broken the > 10-year-old test scripts, which may be hard to fix. Ok, that seems like an acceptable level of "regression" to me. Thanks for explaining. > > regression, I'm not sure net-next is that much "better" than net or > > stable. If a user complains about the new behavior breaking their > > setup, my understanding is that you would likely have to revert the > > patch anyway, or at least add some way to toggle the behavior. > > My hope is that if this patch is applied to net-next without a "Fixes" tag, > users would fix their testing scripts properly. I don't think the lack of a fixes tag will make people fix broken test scripts, but maybe I'm too pessimistic.
Hi, This fix, originally intended for XFRM/IPsec, has been recommended by Steffen Klassert to submit to the net tree. The patch addresses a minor issue related to the IPv4 source address of ICMP error messages, which originated from an old 2011 commit: 415b3334a21a ("icmp: Fix regression in nexthop resolution during replies.") The omission of a "Fixes" tag in the following commit is deliberate to prevent potential test failures and subsequent regression issues that may arise from backporting this patch all stable kerenels. This is a minor fix, anot not security fix. With a seleftest I am submitting this to net-next tree. v2->v3 : fix testscript. The IFS, space, got mangled. v1->v2 : add kernel selftest script Antony Antony (2): xfrm: fix source address in icmp error generation from IPsec gateway selftests/net: add ICMP unreachable over IPsec tunnel net/ipv4/icmp.c | 1 - tools/testing/selftests/net/Makefile | 1 + tools/testing/selftests/net/xfrm_state.sh | 624 ++++++++++++++++++++++ 3 files changed, 625 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/net/xfrm_state.sh -- 2.30.2