netdev - Re: [PATCH v2 net-next 06/16] net: dsa: add a generic procedure for the flow dissector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 26 Sep 2020 20:13:39 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, f.fainelli@...il.com,
        vivien.didelot@...il.com, kuba@...nel.org
Subject: Re: [PATCH v2 net-next 06/16] net: dsa: add a generic procedure for
 the flow dissector

> +void dsa_tag_generic_flow_dissect(const struct sk_buff *skb, __be16 *proto,
> +				  int *offset)
> +{
> +	const struct dsa_device_ops *ops = skb->dev->dsa_ptr->tag_ops;
> +	int tag_len = ops->overhead;
> +
> +	*offset = tag_len;
> +	*proto = ((__be16 *)skb->data)[(tag_len / 2) - 1];
> +}
> +EXPORT_SYMBOL(dsa_tag_generic_flow_dissect);

If you look where this is used:

#if IS_ENABLED(CONFIG_NET_DSA)
                if (unlikely(skb->dev && netdev_uses_dsa(skb->dev) &&
                             proto == htons(ETH_P_XDSA))) {
                        const struct dsa_device_ops *ops;
                        int offset = 0;

                        ops = skb->dev->dsa_ptr->tag_ops;
                        if (ops->flow_dissect &&
                            !ops->flow_dissect(skb, &proto, &offset)) {
                                hlen -= offset;
                                nhoff += offset;
                        }
                }
#endif

We have already done ops = skb->dev->dsa_ptr->tag_ops once. But since
it is in a different compilation unit, the optimise has no idea about
this. So building on my last comment, it would be nice to make this an
inline function to help out the optimiser.

       Andrew