[PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Feng Wang]

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(-)

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

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Leon Romanovsky]

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
> 
> 

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Steffen Klassert]

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.

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Leon Romanovsky]

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.
> >

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Leon Romanovsky]

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

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Leon Romanovsky]

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
> >

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Leon Romanovsky]

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
> > > >
> >

Re: [PATCH] [PATCH ipsec] xfrm: Store ipsec interface index

[Antony Antony]

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