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.
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.
The copypaste probably belongs with the other flow dissector wrappers
in sk_buff.h.
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9254bca2813d..e9e6cf3d148c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -524,12 +524,13 @@ static inline void tun_flow_save_rps_rxhash(struct tun_flow_entry *e, u32 hash)
> */
> static u16 tun_automq_select_queue(struct tun_struct *tun, struct sk_buff *skb)
> {
> + const struct net *net = dev_net(tun->dev);
> struct tun_flow_entry *e;
> u32 txq, numqueues;
>
> numqueues = READ_ONCE(tun->numqueues);
>
> - txq = __skb_get_hash_symmetric(skb);
> + txq = __skb_get_hash_symmetric(net, skb);
> e = tun_flow_find(&tun->flows[tun_hashfn(txq)], txq);
> if (e) {
> tun_flow_save_rps_rxhash(e, txq);
> @@ -1038,10 +1039,11 @@ static void tun_automq_xmit(struct tun_struct *tun, struct sk_buff *skb)
> /* Select queue was not called for the skbuff, so we extract the
> * RPS hash and save it into the flow_table here.
> */
> + const struct net *net = dev_net(tun->dev);
> struct tun_flow_entry *e;
> __u32 rxhash;
>
> - rxhash = __skb_get_hash_symmetric(skb);
> + rxhash = __skb_get_hash_symmetric(net, skb);
> e = tun_flow_find(&tun->flows[tun_hashfn(rxhash)], rxhash);
> if (e)
> tun_flow_save_rps_rxhash(e, rxhash);
> @@ -1938,7 +1940,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> */
> if (!rcu_access_pointer(tun->steering_prog) && tun->numqueues > 1 &&
> !tfile->detached)
> - rxhash = __skb_get_hash_symmetric(skb);
> + rxhash = __skb_get_hash_symmetric(dev_net(tun->dev), skb);
>
> rcu_read_lock();
> if (unlikely(!(tun->dev->flags & IFF_UP))) {
> @@ -2521,7 +2523,7 @@ static int tun_xdp_one(struct tun_struct *tun,
>
> if (!rcu_dereference(tun->steering_prog) && tun->numqueues > 1 &&
> !tfile->detached)
> - rxhash = __skb_get_hash_symmetric(skb);
> + rxhash = __skb_get_hash_symmetric(dev_net(tun->dev), skb);
>
> if (tfile->napi_enabled) {
> queue = &tfile->sk.sk_write_queue;
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 1c2902eaebd3..60a4dc5586c8 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1493,7 +1493,7 @@ __skb_set_sw_hash(struct sk_buff *skb, __u32 hash, bool is_l4)
> }
>
> void __skb_get_hash(struct sk_buff *skb);
> -u32 __skb_get_hash_symmetric(const struct sk_buff *skb);
> +u32 __skb_get_hash_symmetric(const struct net *net, const struct sk_buff *skb);
> u32 skb_get_poff(const struct sk_buff *skb);
> u32 __skb_get_poff(const struct sk_buff *skb, const void *data,
> const struct flow_keys_basic *keys, int hlen);
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index f82e9a7d3b37..634896129780 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -1831,14 +1831,14 @@ EXPORT_SYMBOL(make_flow_keys_digest);
>
> static struct flow_dissector flow_keys_dissector_symmetric __read_mostly;
>
> -u32 __skb_get_hash_symmetric(const struct sk_buff *skb)
> +u32 __skb_get_hash_symmetric(const struct net *net, const struct sk_buff *skb)
> {
> struct flow_keys keys;
>
> __flow_hash_secret_init();
>
> memset(&keys, 0, sizeof(keys));
> - __skb_flow_dissect(NULL, skb, &flow_keys_dissector_symmetric,
> + __skb_flow_dissect(net, skb, &flow_keys_dissector_symmetric,
> &keys, NULL, 0, 0, 0, 0);
>
> return __flow_hash_from_keys(&keys, &hashrnd);
> diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
> index 92d47e469204..3e7296ed5319 100644
> --- a/net/netfilter/nft_hash.c
> +++ b/net/netfilter/nft_hash.c
> @@ -51,7 +51,8 @@ static void nft_symhash_eval(const struct nft_expr *expr,
> struct sk_buff *skb = pkt->skb;
> u32 h;
>
> - h = reciprocal_scale(__skb_get_hash_symmetric(skb), priv->modulus);
> + h = reciprocal_scale(__skb_get_hash_symmetric(nft_net(pkt), skb),
> + priv->modulus);
>
> regs->data[priv->dreg] = h + priv->offset;
> }
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 964225580824..0e6166784972 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1084,7 +1084,8 @@ static int clone(struct datapath *dp, struct sk_buff *skb,
> !dont_clone_flow_key);
> }
>
> -static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
> +static void execute_hash(const struct net *net,
> + struct sk_buff *skb, struct sw_flow_key *key,
> const struct nlattr *attr)
> {
> struct ovs_action_hash *hash_act = nla_data(attr);
> @@ -1097,7 +1098,7 @@ static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
> /* OVS_HASH_ALG_SYM_L4 hashing type. NOTE: this doesn't
> * extend past an encapsulated header.
> */
> - hash = __skb_get_hash_symmetric(skb);
> + hash = __skb_get_hash_symmetric(net, skb);
> }
>
> hash = jhash_1word(hash, hash_act->hash_basis);
> @@ -1359,7 +1360,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> break;
>
> case OVS_ACTION_ATTR_HASH:
> - execute_hash(skb, key, a);
> + execute_hash(ovs_dp_get_net(dp), skb, key, a);
> break;
>
> case OVS_ACTION_ATTR_PUSH_MPLS: {
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index ea3ebc160e25..b047fdb0c02c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1364,7 +1364,9 @@ static unsigned int fanout_demux_hash(struct packet_fanout *f,
> struct sk_buff *skb,
> unsigned int num)
> {
> - return reciprocal_scale(__skb_get_hash_symmetric(skb), num);
> + struct net *net = read_pnet(&f->net);
> +
> + return reciprocal_scale(__skb_get_hash_symmetric(net, skb), num);
> }
>
> static unsigned int fanout_demux_lb(struct packet_fanout *f,
