ipsec: set interface ID different from 0 by tormath1 · Pull Request #18789 · cilium/cilium

tormath1 · 2022-02-11T17:21:21Z

in this patch:
https://patchwork.kernel.org/project/netdevbpf/patch/20220106093606.3046771-6-steffen.klassert@secunet.com/
we see that if_id must be different from 0 for policy and
state construction.

With a 0 value, it makes the creation of the dummy interface fail with
the following error:

level=fatal msg="IPSec with tunneling requires support for xfrm state output masks (Linux 4.19 or later)." error="invalid argument" subsys=daemon

Related-To: flatcar/Flatcar#626
Signed-off-by: Mathieu Tortuyaux mtortuyaux@microsoft.com

tormath1 · 2022-02-14T09:24:28Z

Hi, I see this test is failing: "ConformanceKind1.19 / installation-and-connectivity" is that a flaky test or something needs to be investigated ?

tormath1 · 2022-02-14T10:12:17Z

Thanks @pchaigno for the new run,it is still failing. Investigating on the cilium logs

pkg/datapath/linux/ipsec/ipsec_linux.go

pchaigno · 2022-02-14T22:26:52Z

The failure in privileged unit tests (runtime Jenkins job) seems related:

	 START: ipsec_linux_test.go:120: IPSecSuitePrivileged.TestUpsertIPSecEndpoint
	 START: ipsec_linux_test.go:37: IPSecSuitePrivileged.SetUpTest
	 PASS: ipsec_linux_test.go:37: IPSecSuitePrivileged.SetUpTest	0.000s
	 
	 ipsec_linux_test.go:175:
	     c.Assert(err, IsNil)
	 ... value syscall.Errno = 0x2 ("no such file or directory")
	 
	 START: ipsec_linux_test.go:42: IPSecSuitePrivileged.TearDownTest
	 PASS: ipsec_linux_test.go:42: IPSecSuitePrivileged.TearDownTest	0.000s
	 
	 FAIL: ipsec_linux_test.go:120: IPSecSuitePrivileged.TestUpsertIPSecEndpoint

tormath1 · 2022-02-15T10:35:21Z

@pchaigno I don't think it's a flaky test. I can "reproduce" the "no such file or directory" error. I think it's related to the change on the policy creation: the Ifid is now different from 0 on the policy itself to avoid a mismatch. So policies are created with Ifid = 1 which leads to a failure when the policy is trying to be Get in the test:

cilium/pkg/datapath/linux/ipsec/ipsec_linux_test.go

Lines 166 to 175 in 7350969

    
           toProxyPolicy, err := netlink.XfrmPolicyGet(&netlink.XfrmPolicy{ 
        
           	Src: remote, 
        
           	Dst: local, 
        
           	Dir: netlink.XFRM_DIR_IN, 
        
           	Mark: &netlink.XfrmMark{ 
        
           		Mask:  linux_defaults.IPsecMarkMaskIn, 
        
           		Value: linux_defaults.RouteMarkToProxy, 
        
           	}, 
        
           }) 
        
           c.Assert(err, IsNil)

The &netlink.XfrmPolicy{...} will have an Ifid != 1 so the policy is not found and the test is failing.

tormath1 · 2022-02-15T16:56:03Z

@pchaigno not sure if I can re-trigger the test myself ?

tormath1 · 2022-02-21T12:06:58Z

@pchaigno rebased the branch on master and squashed fixup! commits. 🙂

in this patch: https://patchwork.kernel.org/project/netdevbpf/patch/20220106093606.3046771-6-steffen.klassert@secunet.com/ we see that `if_id` must be different from 0 for policy and state construction. With a 0 value, it makes the creation of the dummy interface fail with the following error: ``` level=fatal msg="IPSec with tunneling requires support for xfrm state output masks (Linux 4.19 or later)." error="invalid argument" subsys=daemon ``` Related-To: flatcar/Flatcar#626 Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>

pchaigno · 2022-02-24T17:56:30Z

/test

This reverts commit 68ac0f3 because ID 0 was meant to be used for configuring the policy/state without matching for a specific interface (e.g., Cilium is affected, see cilium/cilium#18789 and cilium/cilium#19019). Signed-off-by: Kai Lueke <kailueke@linux.microsoft.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Link: https://lore.kernel.org/netdev/20220309130839.3263912-3-steffen.klassert@secunet.com/ (cherry picked from commit a3d9001 https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a3d9001b4e287fc043e5539d03d71a32ab114bcb) Bug: 221187056 Test: run_net_test.sh Change-Id: Iccaf42f776cad9166f808c490bfce586f850554c

This reverts commit 68ac0f3810e76a853b5f7b90601a05c3048b8b54 because ID 0 was meant to be used for configuring the policy/state without matching for a specific interface (e.g., Cilium is affected, see cilium/cilium#18789 and cilium/cilium#19019). Signed-off-by: Kai Lueke <kailueke@linux.microsoft.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Link: https://lore.kernel.org/netdev/20220309130839.3263912-3-steffen.klassert@secunet.com/ (cherry picked from commit a3d9001b4e287fc043e5539d03d71a32ab114bcb https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a3d9001b4e287fc043e5539d03d71a32ab114bcb) Bug: 221187056 Test: run_net_test.sh Change-Id: Iccaf42f776cad9166f808c490bfce586f850554c Signed-off-by: Tashfin Shakeer Rhythm <tashfinshakeerrhythm@gmail.com>

This reverts commit 68ac0f3810e76a853b5f7b90601a05c3048b8b54 because ID 0 was meant to be used for configuring the policy/state without matching for a specific interface (e.g., Cilium is affected, see cilium/cilium#18789 and cilium/cilium#19019). Signed-off-by: Kai Lueke <kailueke@linux.microsoft.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Link: https://lore.kernel.org/netdev/20220309130839.3263912-3-steffen.klassert@secunet.com/ (cherry picked from commit a3d9001b4e287fc043e5539d03d71a32ab114bcb https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a3d9001b4e287fc043e5539d03d71a32ab114bcb) Bug: 221187056 Test: run_net_test.sh Change-Id: Iccaf42f776cad9166f808c490bfce586f850554c

commit a3d9001 upstream. This reverts commit 68ac0f3 because ID 0 was meant to be used for configuring the policy/state without matching for a specific interface (e.g., Cilium is affected, see cilium/cilium#18789 and cilium/cilium#19019). Signed-off-by: Kai Lueke <kailueke@linux.microsoft.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 11, 2022

pchaigno added area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. kind/bug This is a bug in the Cilium logic. needs-backport/1.11 release-note/bug This PR fixes an issue in a previous release of Cilium. labels Feb 11, 2022

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 11, 2022

tormath1 marked this pull request as ready for review February 11, 2022 17:58

tormath1 requested review from a team and kkourt February 11, 2022 17:58

maintainer-s-little-helper bot assigned kkourt Feb 11, 2022

pchaigno reviewed Feb 14, 2022

View reviewed changes

pkg/datapath/linux/ipsec/ipsec_linux.go Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 14, 2022

This comment was marked as resolved.

Sign in to view

tormath1 force-pushed the tormath1/xfrm branch from 6d85f3b to 3b201e4 Compare February 21, 2022 09:07

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 21, 2022

tormath1 force-pushed the tormath1/xfrm branch from 3b201e4 to c39ce70 Compare February 24, 2022 17:21

pchaigno approved these changes Feb 24, 2022

View reviewed changes

pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 26, 2022

kkourt approved these changes Feb 28, 2022

View reviewed changes

kaworu approved these changes Feb 28, 2022

View reviewed changes

kaworu merged commit 735ac6a into cilium:master Feb 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipsec: set interface ID different from 0#18789

ipsec: set interface ID different from 0#18789
kaworu merged 1 commit intocilium:masterfrom
tormath1:tormath1/xfrm

tormath1 commented Feb 11, 2022 •

edited

Loading

Uh oh!

tormath1 commented Feb 14, 2022

Uh oh!

tormath1 commented Feb 14, 2022

Uh oh!

Uh oh!

This comment was marked as resolved.

pchaigno commented Feb 14, 2022

Uh oh!

tormath1 commented Feb 15, 2022

Uh oh!

This comment was marked as resolved.

tormath1 commented Feb 15, 2022

Uh oh!

tormath1 commented Feb 21, 2022

Uh oh!

pchaigno commented Feb 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

tormath1 commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tormath1 commented Feb 14, 2022

Uh oh!

tormath1 commented Feb 14, 2022

Uh oh!

Uh oh!

This comment was marked as resolved.

pchaigno commented Feb 14, 2022

Uh oh!

tormath1 commented Feb 15, 2022

Uh oh!

This comment was marked as resolved.

tormath1 commented Feb 15, 2022

Uh oh!

tormath1 commented Feb 21, 2022

Uh oh!

pchaigno commented Feb 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tormath1 commented Feb 11, 2022 •

edited

Loading