| Message ID | 20241119220411.2961121-1-wangfe@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | xfrm: add SA information to the offloaded packet when if_id is set | expand |
Hi, On 11/19/24 23:04, Feng Wang wrote: > From: wangfe <wangfe@google.com> Unneeded, since the author (you) matches the submitter email address (yours). BTW please include a patch revision number into the subj prefix to help reviewers. > @@ -240,6 +256,7 @@ bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb) > struct xfrm_state *xs; > struct nsim_sa *tsa; > u32 sa_idx; > + struct xfrm_offload *xo; This is network driver code, please respect the reverse x-mas tree order above. > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c > index e5722c95b8bb..59ac45f0c4ac 100644 > --- a/net/xfrm/xfrm_output.c > +++ b/net/xfrm/xfrm_output.c > @@ -706,6 +706,8 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb) > struct xfrm_state *x = skb_dst(skb)->xfrm; > int family; > int err; > + struct xfrm_offload *xo; > + struct sec_path *sp; I see the xfrm subtree is more relaxed with the reverse x-mas tree order, but for consistency I would respect it even here. Cheers, Paolo
Hi Paolo, Thanks for your comments, I will upload a new patch to address your comments soon. Feng On Thu, Nov 21, 2024 at 12:09 AM Paolo Abeni <pabeni@redhat.com> wrote: > > Hi, > > On 11/19/24 23:04, Feng Wang wrote: > > From: wangfe <wangfe@google.com> > > Unneeded, since the author (you) matches the submitter email address > (yours). > > BTW please include a patch revision number into the subj prefix to help > reviewers. > > > @@ -240,6 +256,7 @@ bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb) > > struct xfrm_state *xs; > > struct nsim_sa *tsa; > > u32 sa_idx; > > + struct xfrm_offload *xo; > > This is network driver code, please respect the reverse x-mas tree order > above. > > > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c > > index e5722c95b8bb..59ac45f0c4ac 100644 > > --- a/net/xfrm/xfrm_output.c > > +++ b/net/xfrm/xfrm_output.c > > @@ -706,6 +706,8 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb) > > struct xfrm_state *x = skb_dst(skb)->xfrm; > > int family; > > int err; > > + struct xfrm_offload *xo; > > + struct sec_path *sp; > > I see the xfrm subtree is more relaxed with the reverse x-mas tree > order, but for consistency I would respect it even here. > > Cheers, > > Paolo >
diff --git a/drivers/net/netdevsim/ipsec.c b/drivers/net/netdevsim/ipsec.c index 88187dd4eb2d..fd460f456ab7 100644 --- a/drivers/net/netdevsim/ipsec.c +++ b/drivers/net/netdevsim/ipsec.c @@ -153,7 +153,8 @@ static int nsim_ipsec_add_sa(struct xfrm_state *xs, return -EINVAL; } - if (xs->xso.type != XFRM_DEV_OFFLOAD_CRYPTO) { + if (xs->xso.type != XFRM_DEV_OFFLOAD_CRYPTO && + xs->xso.type != XFRM_DEV_OFFLOAD_PACKET) { NL_SET_ERR_MSG_MOD(extack, "Unsupported ipsec offload type"); return -EINVAL; } @@ -169,6 +170,7 @@ static int nsim_ipsec_add_sa(struct xfrm_state *xs, memset(&sa, 0, sizeof(sa)); sa.used = true; sa.xs = xs; + sa.if_id = xs->if_id; if (sa.xs->id.proto & IPPROTO_ESP) sa.crypt = xs->ealg || xs->aead; @@ -227,10 +229,24 @@ static bool nsim_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs) return true; } +static int nsim_ipsec_add_policy(struct xfrm_policy *policy, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static void nsim_ipsec_del_policy(struct xfrm_policy *policy) +{ +} + static const struct xfrmdev_ops nsim_xfrmdev_ops = { .xdo_dev_state_add = nsim_ipsec_add_sa, .xdo_dev_state_delete = nsim_ipsec_del_sa, .xdo_dev_offload_ok = nsim_ipsec_offload_ok, + + .xdo_dev_policy_add = nsim_ipsec_add_policy, + .xdo_dev_policy_delete = nsim_ipsec_del_policy, + }; bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb) @@ -240,6 +256,7 @@ bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb) struct xfrm_state *xs; struct nsim_sa *tsa; u32 sa_idx; + struct xfrm_offload *xo; /* do we even need to check this packet? */ if (!sp) @@ -275,6 +292,19 @@ bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb) return false; } + if (xs->if_id) { + if (xs->if_id != tsa->if_id) { + netdev_err(ns->netdev, "unmatched if_id %d %d\n", + xs->if_id, tsa->if_id); + return false; + } + xo = xfrm_offload(skb); + if (!xo || !(xo->flags & XFRM_XMIT)) { + netdev_err(ns->netdev, "offload flag missing or wrong\n"); + return false; + } + } + ipsec->tx++; return true; diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h index bf02efa10956..4941b6e46d0a 100644 --- a/drivers/net/netdevsim/netdevsim.h +++ b/drivers/net/netdevsim/netdevsim.h @@ -41,6 +41,7 @@ struct nsim_sa { __be32 ipaddr[4]; u32 key[4]; u32 salt; + u32 if_id; bool used; bool crypt; bool rx; diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index e5722c95b8bb..59ac45f0c4ac 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -706,6 +706,8 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb) struct xfrm_state *x = skb_dst(skb)->xfrm; int family; int err; + struct xfrm_offload *xo; + struct sec_path *sp; family = (x->xso.type != XFRM_DEV_OFFLOAD_PACKET) ? x->outer_mode.family : skb_dst(skb)->ops->family; @@ -728,7 +730,27 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb) kfree_skb(skb); return -EHOSTUNREACH; } + if (x->if_id) { + 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); + xo = xfrm_offload(skb); + if (!xo) { + secpath_reset(skb); + XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR); + kfree_skb(skb); + return -EINVAL; + } + xo->flags |= XFRM_XMIT; + } return xfrm_output_resume(sk, skb, 0); }