diff mbox series

[ipsec] xfrm: Store ipsec interface index

Message ID 20240318231328.2086239-1-wangfe@google.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [ipsec] xfrm: Store ipsec interface index | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 939 this patch: 939
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 3 maintainers not CCed: pabeni@redhat.com edumazet@google.com kuba@kernel.org
netdev/build_clang success Errors and warnings before: 956 this patch: 956
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 956 this patch: 956
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest warning net-next-2024-03-19--03-00 (tests: 906)

Commit Message

Feng Wang March 18, 2024, 11:13 p.m. UTC
From: wangfe <wangfe@google.com>

When there are multiple ipsec sessions, packet offload driver
can use the index to distinguish the packets from the different
sessions even though xfrm_selector are same. Thus each packet is
handled corresponding to its session parameter.

Signed-off-by: wangfe <wangfe@google.com>
---
 net/xfrm/xfrm_interface_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Leon Romanovsky March 19, 2024, 8:42 a.m. UTC | #1
On Mon, Mar 18, 2024 at 04:13:28PM -0700, Feng Wang wrote:
> From: wangfe <wangfe@google.com>
> 
> When there are multiple ipsec sessions, packet offload driver
> can use the index to distinguish the packets from the different
> sessions even though xfrm_selector are same. 

Do we have such "packet offload driver" in the kernel tree?

Thanks

> Thus each packet is handled corresponding to its session parameter.
> 
> Signed-off-by: wangfe <wangfe@google.com>
> ---
>  net/xfrm/xfrm_interface_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
> index 21d50d75c260..996571af53e5 100644
> --- a/net/xfrm/xfrm_interface_core.c
> +++ b/net/xfrm/xfrm_interface_core.c
> @@ -506,7 +506,9 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
>  	xfrmi_scrub_packet(skb, !net_eq(xi->net, dev_net(dev)));
>  	skb_dst_set(skb, dst);
>  	skb->dev = tdev;
> -
> +#ifdef CONFIG_XFRM_OFFLOAD
> +	skb->skb_iif = if_id;
> +#endif
>  	err = dst_output(xi->net, skb->sk, skb);
>  	if (net_xmit_eval(err) == 0) {
>  		dev_sw_netstats_tx_add(dev, 1, length);
> -- 
> 2.44.0.291.gc1ea87d7ee-goog
> 
>
Steffen Klassert March 20, 2024, 4:33 a.m. UTC | #2
On Tue, Mar 19, 2024 at 10:15:13AM -0700, Feng Wang wrote:
> Hi Leon,
> 
> There is no "packet offload driver" in the current kernel tree.  The packet
> offload driver mostly is vendor specific, it implements hardware packet
> offload.

There are 'packet offload drivers' in the kernel, that's why we
support this kind of offload. We don't add code for proprietary
drivers.

> On Tue, Mar 19, 2024 at 1:42 AM Leon Romanovsky <leon@kernel.org> wrote:
> 
> > On Mon, Mar 18, 2024 at 04:13:28PM -0700, Feng Wang wrote:
> > > From: wangfe <wangfe@google.com>
> > >
> > > When there are multiple ipsec sessions, packet offload driver
> > > can use the index to distinguish the packets from the different
> > > sessions even though xfrm_selector are same.
> >
> > Do we have such "packet offload driver" in the kernel tree?
> >
> > Thanks
> >
> > > Thus each packet is handled corresponding to its session parameter.
> > >
> > > Signed-off-by: wangfe <wangfe@google.com>
> > > ---
> > >  net/xfrm/xfrm_interface_core.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/xfrm/xfrm_interface_core.c
> > b/net/xfrm/xfrm_interface_core.c
> > > index 21d50d75c260..996571af53e5 100644
> > > --- a/net/xfrm/xfrm_interface_core.c
> > > +++ b/net/xfrm/xfrm_interface_core.c
> > > @@ -506,7 +506,9 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device
> > *dev, struct flowi *fl)
> > >       xfrmi_scrub_packet(skb, !net_eq(xi->net, dev_net(dev)));
> > >       skb_dst_set(skb, dst);
> > >       skb->dev = tdev;
> > > -
> > > +#ifdef CONFIG_XFRM_OFFLOAD
> > > +     skb->skb_iif = if_id;
> > > +#endif

This looks wrong. The network interface ID is not the same as the xfrm
interface ID.
Leon Romanovsky March 21, 2024, 9:32 a.m. UTC | #3
On Wed, Mar 20, 2024 at 11:05:13AM -0700, Feng Wang wrote:
> Hi Steffen,
> 
> Thanks for your comment.  Firstly,  the patch is using the xfrm interface
> ID instead of network interface ID. Secondly, would you please point me to
> the 'packet offload drivers' in the kernel tree?

First, please don't reply to emails in top-post format.
Second, did you try to search for "packet offload drivers" in the kernel?
https://elixir.bootlin.com/linux/v6.8.1/source/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c#L1152

Thanks

> I want to understand how the offload driver can distinguish 2 ipsec
> sessions if two sessions accidentally have the same address/mask and proto
> values(same xfrm_selector)? The offload driver needs to find the
> corresponding encryption parameters to do the work.
> 
> Thank you for your help,
> 
> Feng
> 
> 
> 
> On Tue, Mar 19, 2024 at 9:33 PM Steffen Klassert <
> steffen.klassert@secunet.com> wrote:
> 
> > On Tue, Mar 19, 2024 at 10:15:13AM -0700, Feng Wang wrote:
> > > Hi Leon,
> > >
> > > There is no "packet offload driver" in the current kernel tree.  The
> > packet
> > > offload driver mostly is vendor specific, it implements hardware packet
> > > offload.
> >
> > There are 'packet offload drivers' in the kernel, that's why we
> > support this kind of offload. We don't add code for proprietary
> > drivers.
> >
> > > On Tue, Mar 19, 2024 at 1:42 AM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > > On Mon, Mar 18, 2024 at 04:13:28PM -0700, Feng Wang wrote:
> > > > > From: wangfe <wangfe@google.com>
> > > > >
> > > > > When there are multiple ipsec sessions, packet offload driver
> > > > > can use the index to distinguish the packets from the different
> > > > > sessions even though xfrm_selector are same.
> > > >
> > > > Do we have such "packet offload driver" in the kernel tree?
> > > >
> > > > Thanks
> > > >
> > > > > Thus each packet is handled corresponding to its session parameter.
> > > > >
> > > > > Signed-off-by: wangfe <wangfe@google.com>
> > > > > ---
> > > > >  net/xfrm/xfrm_interface_core.c | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/xfrm/xfrm_interface_core.c
> > > > b/net/xfrm/xfrm_interface_core.c
> > > > > index 21d50d75c260..996571af53e5 100644
> > > > > --- a/net/xfrm/xfrm_interface_core.c
> > > > > +++ b/net/xfrm/xfrm_interface_core.c
> > > > > @@ -506,7 +506,9 @@ xfrmi_xmit2(struct sk_buff *skb, struct
> > net_device
> > > > *dev, struct flowi *fl)
> > > > >       xfrmi_scrub_packet(skb, !net_eq(xi->net, dev_net(dev)));
> > > > >       skb_dst_set(skb, dst);
> > > > >       skb->dev = tdev;
> > > > > -
> > > > > +#ifdef CONFIG_XFRM_OFFLOAD
> > > > > +     skb->skb_iif = if_id;
> > > > > +#endif
> >
> > This looks wrong. The network interface ID is not the same as the xfrm
> > interface ID.
> >
Leon Romanovsky April 1, 2024, 2:27 p.m. UTC | #4
On Fri, Mar 22, 2024 at 12:14:44PM -0700, Feng Wang wrote:
> Hi Leon and Steffen,
> 
> Thanks for providing me with the information. I went through the offload
> driver code but I didn't find any solution for my case.  Is there any
> existing solution available?  For example, there are 2 IPSec sessions with
> the same xfrm_selector results, when trying to encrypt the packet, how to
> find out which session this packet belongs to?

HW catches packets based on match criteria of source and destination. If
source, destination and other match criteria are the same for different
sessions, then from HW perspective, it is the same session.

Thanks
Leon Romanovsky April 2, 2024, 7:51 a.m. UTC | #5
On Mon, Apr 01, 2024 at 11:09:41AM -0700, Feng Wang wrote:
> Thanks Leon for answering my question.  In the above example, if we can
> pass the xfrm interface id to the HW, then HW can distinguish them based on
> it. That's what my patch is trying to do.

From partial grep, it looks like "xfrm interface id" is actually netdevice
index. If this is the case, HW doesn't need to know about it, because
packet offload is performed by specific device and skb_iif will be equal
to that index anyway.

> Would you please take this into consideration? If needed, I can improve my
> patch.

As a standalone patch, it is not correct. If you have a real use case,
please send together with code which uses it.

Thanks

> 
> Thanks,
> 
> Feng
> 
> 
> 
> 
> On Mon, Apr 1, 2024 at 7:27 AM Leon Romanovsky <leon@kernel.org> wrote:
> 
> > On Fri, Mar 22, 2024 at 12:14:44PM -0700, Feng Wang wrote:
> > > Hi Leon and Steffen,
> > >
> > > Thanks for providing me with the information. I went through the offload
> > > driver code but I didn't find any solution for my case.  Is there any
> > > existing solution available?  For example, there are 2 IPSec sessions
> > with
> > > the same xfrm_selector results, when trying to encrypt the packet, how to
> > > find out which session this packet belongs to?
> >
> > HW catches packets based on match criteria of source and destination. If
> > source, destination and other match criteria are the same for different
> > sessions, then from HW perspective, it is the same session.
> >
> > Thanks
> >
Leon Romanovsky April 3, 2024, 6:45 a.m. UTC | #6
On Tue, Apr 02, 2024 at 02:10:16PM -0700, Feng Wang wrote:
> The xfrm interface ID is the index of the ipsec device, for example,
> ipsec11, ipsec12.  One ipsec application(VPN) might create an ipsec11
> interface and send the data through this interface.
> Another application(Wifi calling) might create an ipsec12 interface and
> send its data through ipsec12.  Both packets are routed through the kernel
> to the one device driver(wifi).  When the device driver receives the
> packet, it needs to find the correct application parameters to encrypt the
> packet.  So if the skb_iif is marked by the kernel with ipsec11 or
> ipsec12,  device driver can use this information to find the corresponding
> parameter.  I hope I explain my user case clearly.  If there is any
> misunderstanding, please let me know.  I try my best to make it clear.

Like I said before, please send the code which uses this feature. Right
now, packet offload doesn't need this feature.

Thanks

> 
> Thanks Leon.
> Feng
> 
> 
> On Tue, Apr 2, 2024 at 12:51 AM Leon Romanovsky <leon@kernel.org> wrote:
> 
> > On Mon, Apr 01, 2024 at 11:09:41AM -0700, Feng Wang wrote:
> > > Thanks Leon for answering my question.  In the above example, if we can
> > > pass the xfrm interface id to the HW, then HW can distinguish them based
> > on
> > > it. That's what my patch is trying to do.
> >
> > From partial grep, it looks like "xfrm interface id" is actually netdevice
> > index. If this is the case, HW doesn't need to know about it, because
> > packet offload is performed by specific device and skb_iif will be equal
> > to that index anyway.
> >
> > > Would you please take this into consideration? If needed, I can improve
> > my
> > > patch.
> >
> > As a standalone patch, it is not correct. If you have a real use case,
> > please send together with code which uses it.
> >
> > Thanks
> >
> > >
> > > Thanks,
> > >
> > > Feng
> > >
> > >
> > >
> > >
> > > On Mon, Apr 1, 2024 at 7:27 AM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > > On Fri, Mar 22, 2024 at 12:14:44PM -0700, Feng Wang wrote:
> > > > > Hi Leon and Steffen,
> > > > >
> > > > > Thanks for providing me with the information. I went through the
> > offload
> > > > > driver code but I didn't find any solution for my case.  Is there any
> > > > > existing solution available?  For example, there are 2 IPSec sessions
> > > > with
> > > > > the same xfrm_selector results, when trying to encrypt the packet,
> > how to
> > > > > find out which session this packet belongs to?
> > > >
> > > > HW catches packets based on match criteria of source and destination.
> > If
> > > > source, destination and other match criteria are the same for different
> > > > sessions, then from HW perspective, it is the same session.
> > > >
> > > > Thanks
> > > >
> >
Antony Antony April 5, 2024, 2:19 p.m. UTC | #7
Hi Feng,

On Wed, Apr 03, 2024 at 09:45:07AM +0300, Leon Romanovsky wrote:
> On Tue, Apr 02, 2024 at 02:10:16PM -0700, Feng Wang wrote:
> > The xfrm interface ID is the index of the ipsec device, for example,
> > ipsec11, ipsec12.  One ipsec application(VPN) might create an ipsec11
> > interface and send the data through this interface.

Where to find xfrm interface if_id depends on the direction the packet is 
traversing, direction "out": (clear text in ESP out), or "in": (ESP in clear 
text out) use if_id diffrently.

Which case are you looking at? It sounds like out.

> > Another application(Wifi calling) might create an ipsec12 interface and
> > send its data through ipsec12.  Both packets are routed through the kernel
> > to the one device driver(wifi).  When the device driver receives the
> > packet, it needs to find the correct application parameters to encrypt the

this looks like an out case. After a successful dst lookup, xfrm_lookup(),

look at skb_dst(skb)->xfrm->if_id ?

> > packet.  So if the skb_iif is marked by the kernel with ipsec11 or
> > ipsec12,  device driver can use this information to find the corresponding
> > parameter.  I hope I explain my user case clearly.  If there is any
> > misunderstanding, please let me know.  I try my best to make it clear.

skb_dst(skb)->xfrm->if_id should match  what is in the xfrm policy I think,
p->if_id.

Note I assumed the packet is locally generated. If it is a forwarded packet 
there could be another policy lookup before.
 
> Like I said before, please send the code which uses this feature. Right
> now, packet offload doesn't need this feature.

+1
-antony
diff mbox series

Patch

diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 21d50d75c260..996571af53e5 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -506,7 +506,9 @@  xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
 	xfrmi_scrub_packet(skb, !net_eq(xi->net, dev_net(dev)));
 	skb_dst_set(skb, dst);
 	skb->dev = tdev;
-
+#ifdef CONFIG_XFRM_OFFLOAD
+	skb->skb_iif = if_id;
+#endif
 	err = dst_output(xi->net, skb->sk, skb);
 	if (net_xmit_eval(err) == 0) {
 		dev_sw_netstats_tx_add(dev, 1, length);