Re: [PATCH nf] netfilter: nf_reject: init skb->dev for reset packet

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Florian Westphal wrote:
> Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > Florian Westphal wrote:
> > > Florian Westphal <fw@strlen.de> wrote:
> > > > ... doesn't solve the nft_hash.c issue (which calls _symmetric version, and
> > > > that uses flow_key definiton that isn't exported outside flow_dissector.o.
> > > 
> > > and here is the diff that would pass net for _symmetric, not too
> > > horrible I think.
> > > 
> > > With that and the copypaste of skb_get_hash into nf_trace infra
> > > netfilter can still pass skbs to the flow dissector with NULL skb->sk,dev
> > > but the WARN would no longer trigger as struct net is non-null.
> > 
> > Thanks for coding this up Florian. This overall looks good to me.
> 
> Thanks for reviewing.
> 
> > One suggested change is to introduce a three underscore variant (yes
> > really) ___skb_get_hash_symmetric that takes the optional net, and
> > leave the existing callers of the two underscore version as is.
> 
> Okay, that reduces the code churn.
> 
> > The copypaste probably belongs with the other flow dissector wrappers
> > in sk_buff.h.
> 
> skb_get_hash(skb);
> __skb_get_hash_symmetric(skb);
> ____skb_get_hash_symmetric(net, skb);
> 
> I named the copypasta as nf_skb_get_hash. If placed in sk_buff.h:
> net_get_hash_net()?
> skb_get_hash()?

Still passing an skb too, so skb_get_hash_net()?
 
> And if either of that exists, maybe then use
> skb_get_hash_symmetric_net(net, skb)

If symmetric is equally good for nft, that would be preferable, as it
avoids the extra function. But I suppose it aliases the two flow
directions, which may be exactly what you don't want?

> or similar?
> 
> (There is no skb_get_hash_symmetric, no idea why it
>  uses __prefix).

Perhaps because it is more closely analogous to __skb_get_hash, than
to skb_get_hash.