Hi Nicolas, On Tue, Dec 08, 2020 at 10:02:16AM +0100, Nicolas Dichtel wrote: > Le 07/12/2020 à 14:43, Phil Sutter a écrit : [...] > > diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c > > index aa4cdcf69d471..24af61c95b4d4 100644 > > --- a/net/xfrm/xfrm_interface.c > > +++ b/net/xfrm/xfrm_interface.c > > @@ -317,7 +317,8 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) > > skb_dst_set(skb, dst); > > skb->dev = tdev; > > > > - err = dst_output(xi->net, skb->sk, skb); > > + err = NF_HOOK(skb_dst(skb)->ops->family, NF_INET_LOCAL_OUT, xi->net, > skb->protocol must be correctly set, maybe better to use it instead of > skb_dst(skb)->ops->family? skb->protocol holds ETH_P_* values in network byte order, NF_HOOK() expects an NFPROTO_* value, so this would at least not be a simple replacement. Actually I copied the code from xfrm_output_resume() in xfrm_output.c, where skb_dst(skb)->ops is dereferenced without checking as well. Do you think this is risky? > > + skb->sk, skb, NULL, skb_dst(skb)->dev, dst_output); > And here, tdev instead of skb_dst(skb)->dev ? Well yes, tdev was set to dst->dev earlier. Likewise I could use dst directly instead of skb_dst(skb) to simplify the call a bit further. OTOH I like how in the version above it is clear that skb's dst should be used, irrespective of the code above (and any later changes that may receive). No strong opinion though, so if your version is regarded the preferred one, I'm fine with that. Thanks, Phil
