Florian Westphal wrote: > Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > > Florian Westphal wrote: > > > Florian Westphal <fw@xxxxxxxxx> 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.
