Setting security path with IPsec packet offload mode

Commit Message

Bharat Bhushan June 22, 2023, 6:58 a.m. UTC

Hi All,

Have a query related to security patch (secpath_set()) with packet offload mode on egress side. Working to enable ipsec packet offload while Crypto offload is working.
For packet offload xfrm_offload(*skb) returns false in driver. While looking in xfrm framework, cannot find where security patch (secpath_set()) is set with packet offload mode on egress side.

For sure I might be missing something here and looking for help to understand same. Meantime just tried below hack:


Thanks in advance,
-Bharat

Comments

Leon Romanovsky June 22, 2023, 8:35 a.m. UTC | #1

On Thu, Jun 22, 2023 at 06:58:06AM +0000, Bharat Bhushan wrote:
> Hi All,
> 
> Have a query related to security patch (secpath_set()) with packet offload mode on egress side. Working to enable ipsec packet offload while Crypto offload is working.
> For packet offload xfrm_offload(*skb) returns false in driver. While looking in xfrm framework, cannot find where security patch (secpath_set()) is set with packet offload mode on egress side.

The idea of packet offload is to take plain text packets and perform all needed magic
in HW without need from driver and stack to make anything.

We don't set secpath in TX path as such packets exit from XFRM as "plain" ones toward HW.
It is different in RX, we set there, as XFRM core still needs to perform some code on
the arrived packets, before forwarding them to stack.

Thanks

> 
> For sure I might be missing something here and looking for help to understand same. Meantime just tried below hack:
> 
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index ff114d68cc43..8499c0e74a5a 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -718,12 +718,24 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
>         }
>  
>         if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> +               struct sec_path *sp;
>                 if (!xfrm_dev_offload_ok(skb, x)) {
>                         XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
>                         kfree_skb(skb);
>                         return -EHOSTUNREACH;
>                 }
>  
> +               sp = secpath_set(skb);
> +               if (!sp) {
> +                       XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> +                       kfree_skb(skb);
> +                       return -ENOMEM;
> +               }
> +
> +               sp->olen++;
> +               sp->xvec[sp->len++] = x;
> +               xfrm_state_hold(x);
> +
>                 return xfrm_output_resume(sk, skb, 0);
>         }
>  
> 
> Thanks in advance,
> -Bharat
>

Bharat Bhushan June 23, 2023, 6:48 a.m. UTC | #2

Hi Leon,

> -----Original Message-----
> From: Leon Romanovsky <[e-mail]>
> Sent: Thursday, June 22, 2023 2:05 PM
> To: Bharat Bhushan <[e-mail]>
> Cc: Steffen Klassert <[e-mail]>;
> [e-mail]; David S. Miller <[e-mail]>; Eric
> Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]
> Subject: [EXT] Re: Setting security path with IPsec packet offload mode
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, Jun 22, 2023 at 06:58:06AM +0000, Bharat Bhushan wrote:
> > Hi All,
> >
> > Have a query related to security patch (secpath_set()) with packet offload
> mode on egress side. Working to enable ipsec packet offload while Crypto
> offload is working.
> > For packet offload xfrm_offload(*skb) returns false in driver. While looking in
> xfrm framework, cannot find where security patch (secpath_set()) is set with
> packet offload mode on egress side.
> 
> The idea of packet offload is to take plain text packets and perform all needed
> magic in HW without need from driver and stack to make anything.

So driver does not know whether it normal packet and will be transmitted normally or this will undergo inline ipsec in hardware.

In our case packet transmit requires a different code flow in driver, to pass some extra details in transmit descriptor like state and policy pointers. So is there some way driver can find same and extra state and policy details from skb? 

Thanks
-Bharat

> 
> We don't set secpath in TX path as such packets exit from XFRM as "plain" ones
> toward HW.
> It is different in RX, we set there, as XFRM core still needs to perform some code
> on the arrived packets, before forwarding them to stack.
> 
> Thanks
> 
> >
> > For sure I might be missing something here and looking for help to understand
> same. Meantime just tried below hack:
> >
> > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index
> > ff114d68cc43..8499c0e74a5a 100644
> > --- a/net/xfrm/xfrm_output.c
> > +++ b/net/xfrm/xfrm_output.c
> > @@ -718,12 +718,24 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
> >         }
> >
> >         if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> > +               struct sec_path *sp;
> >                 if (!xfrm_dev_offload_ok(skb, x)) {
> >                         XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> >                         kfree_skb(skb);
> >                         return -EHOSTUNREACH;
> >                 }
> >
> > +               sp = secpath_set(skb);
> > +               if (!sp) {
> > +                       XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> > +                       kfree_skb(skb);
> > +                       return -ENOMEM;
> > +               }
> > +
> > +               sp->olen++;
> > +               sp->xvec[sp->len++] = x;
> > +               xfrm_state_hold(x);
> > +
> >                 return xfrm_output_resume(sk, skb, 0);
> >         }
> >
> >
> > Thanks in advance,
> > -Bharat
> >

Bharat Bhushan June 23, 2023, 7:01 a.m. UTC | #3

> -----Original Message-----
> From: Bharat Bhushan <[e-mail]>
> Sent: Friday, June 23, 2023 12:18 PM
> To: Leon Romanovsky <[e-mail]>
> Cc: Steffen Klassert <[e-mail]>;
> [e-mail]; David S. Miller <[e-mail]>; Eric
> Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>; Paolo
> Abeni <[e-mail]>; [e-mail]
> Subject: RE: [EXT] Re: Setting security path with IPsec packet offload mode
> 
> Hi Leon,
> 
> > -----Original Message-----
> > From: Leon Romanovsky <[e-mail]>
> > Sent: Thursday, June 22, 2023 2:05 PM
> > To: Bharat Bhushan <[e-mail]>
> > Cc: Steffen Klassert <[e-mail]>;
> > [e-mail]; David S. Miller <[e-mail]>;
> > Eric Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>;
> > Paolo Abeni <[e-mail]>; [e-mail]
> > Subject: [EXT] Re: Setting security path with IPsec packet offload
> > mode
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Thu, Jun 22, 2023 at 06:58:06AM +0000, Bharat Bhushan wrote:
> > > Hi All,
> > >
> > > Have a query related to security patch (secpath_set()) with packet
> > > offload
> > mode on egress side. Working to enable ipsec packet offload while
> > Crypto offload is working.
> > > For packet offload xfrm_offload(*skb) returns false in driver. While
> > > looking in
> > xfrm framework, cannot find where security patch (secpath_set()) is
> > set with packet offload mode on egress side.
> >
> > The idea of packet offload is to take plain text packets and perform
> > all needed magic in HW without need from driver and stack to make anything.
> 
> So driver does not know whether it normal packet and will be transmitted
> normally or this will undergo inline ipsec in hardware.
> 
> In our case packet transmit requires a different code flow in driver, to pass some
> extra details in transmit descriptor like state and policy pointers. So is there some
> way driver can find same and extra state and policy details from skb?

s/extra/extract
I mean extract xfrm state and policy details from skb?

Thanks
-Bharat

> 
> Thanks
> -Bharat
> 
> >
> > We don't set secpath in TX path as such packets exit from XFRM as
> > "plain" ones toward HW.
> > It is different in RX, we set there, as XFRM core still needs to
> > perform some code on the arrived packets, before forwarding them to stack.
> >
> > Thanks
> >
> > >
> > > For sure I might be missing something here and looking for help to
> > > understand
> > same. Meantime just tried below hack:
> > >
> > > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index
> > > ff114d68cc43..8499c0e74a5a 100644
> > > --- a/net/xfrm/xfrm_output.c
> > > +++ b/net/xfrm/xfrm_output.c
> > > @@ -718,12 +718,24 @@ int xfrm_output(struct sock *sk, struct sk_buff
> *skb)
> > >         }
> > >
> > >         if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> > > +               struct sec_path *sp;
> > >                 if (!xfrm_dev_offload_ok(skb, x)) {
> > >                         XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> > >                         kfree_skb(skb);
> > >                         return -EHOSTUNREACH;
> > >                 }
> > >
> > > +               sp = secpath_set(skb);
> > > +               if (!sp) {
> > > +                       XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
> > > +                       kfree_skb(skb);
> > > +                       return -ENOMEM;
> > > +               }
> > > +
> > > +               sp->olen++;
> > > +               sp->xvec[sp->len++] = x;
> > > +               xfrm_state_hold(x);
> > > +
> > >                 return xfrm_output_resume(sk, skb, 0);
> > >         }
> > >
> > >
> > > Thanks in advance,
> > > -Bharat
> > >

Leon Romanovsky June 25, 2023, 7:27 a.m. UTC | #4

On Fri, Jun 23, 2023 at 06:48:21AM +0000, Bharat Bhushan wrote:
> Hi Leon,
> 
> > -----Original Message-----
> > From: Leon Romanovsky <[e-mail]>
> > Sent: Thursday, June 22, 2023 2:05 PM
> > To: Bharat Bhushan <[e-mail]>
> > Cc: Steffen Klassert <[e-mail]>;
> > [e-mail]; David S. Miller <[e-mail]>; Eric
> > Dumazet <[e-mail]>; Jakub Kicinski <[e-mail]>; Paolo
> > Abeni <[e-mail]>; [e-mail]
> > Subject: [EXT] Re: Setting security path with IPsec packet offload mode
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Thu, Jun 22, 2023 at 06:58:06AM +0000, Bharat Bhushan wrote:
> > > Hi All,
> > >
> > > Have a query related to security patch (secpath_set()) with packet offload
> > mode on egress side. Working to enable ipsec packet offload while Crypto
> > offload is working.
> > > For packet offload xfrm_offload(*skb) returns false in driver. While looking in
> > xfrm framework, cannot find where security patch (secpath_set()) is set with
> > packet offload mode on egress side.
> > 
> > The idea of packet offload is to take plain text packets and perform all needed
> > magic in HW without need from driver and stack to make anything.
> 
> So driver does not know whether it normal packet and will be transmitted normally or this will undergo inline ipsec in hardware.

Yes, this is whole idea of packet offload. Such design allows natively
support tunnel and eswitch modes.

> 
> In our case packet transmit requires a different code flow in driver, to pass some extra details in transmit descriptor like state and policy pointers. 

It sounds like existing IPsec crypto mode to me, which does exactly that
- lookup in SW, while crypto in HW.

> So is there some way driver can find same and extra state and policy details from skb? 

I'm not aware, maybe Steffen can answer.

> 
> Thanks
> -Bharat

Thanks

Patch

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index ff114d68cc43..8499c0e74a5a 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -718,12 +718,24 @@  int xfrm_output(struct sock *sk, struct sk_buff *skb)
        }
 
        if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
+               struct sec_path *sp;
                if (!xfrm_dev_offload_ok(skb, x)) {
                        XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
                        kfree_skb(skb);
                        return -EHOSTUNREACH;
                }
 
+               sp = secpath_set(skb);
+               if (!sp) {
+                       XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+                       kfree_skb(skb);
+                       return -ENOMEM;
+               }
+
+               sp->olen++;
+               sp->xvec[sp->len++] = x;
+               xfrm_state_hold(x);
+
                return xfrm_output_resume(sk, skb, 0);
        }