Hi Andrew,
On 03/31/2017 09:06 AM, Andrew Lunn wrote:
> Hi Florian
>
>> +static enum dsa_tag_protocol dsa_loop_get_protocol(struct dsa_switch *ds)
>> +{
>> + dev_dbg(ds->dev, "%s\n", __func__);
>> +
>> + return DSA_TAG_PROTO_NONE;
>> +}
>
> I'm wondering how safe this is:
>
> static const struct dsa_device_ops none_ops = {
> .xmit = dsa_slave_notag_xmit,
> .rcv = NULL,
> };
>
> /*
> * If the CPU connects to this switch, set the switch tree
> * tagging protocol to the preferred tagging format of this
> * switch.
> */
> if (dst->cpu_switch == ds) {
> enum dsa_tag_protocol tag_protocol;
>
> tag_protocol = ops->get_tag_protocol(ds);
> dst->tag_ops = dsa_resolve_tag_protocol(tag_protocol);
> if (IS_ERR(dst->tag_ops))
> return PTR_ERR(dst->tag_ops);
>
> dst->rcv = dst->tag_ops->rcv;
> }
>
>
> static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
> struct packet_type *pt, struct net_device *orig_dev)
> {
> struct dsa_switch_tree *dst = dev->dsa_ptr;
>
> if (unlikely(dst == NULL)) {
> kfree_skb(skb);
> return 0;
> }
>
> return dst->rcv(skb, dev, pt, orig_dev);
> }
>
> static struct packet_type dsa_pack_type __read_mostly = {
> .type = cpu_to_be16(ETH_P_XDSA),
> .func = dsa_switch_rcv,
> };
>
> It looks like when a frame is received, we are going to dereference a
> NULL pointer.
Actually we do not, because netdev_uses_dsa() returns true only when
dst->rcv is different from NULL. When dst->rcv is NULL we completely
bypass the DSA hook in eth_type_trans() and everything is well, the
master network device is the one receiving packets.
This is actually the intended behavior for netdev_uses_dsa() because it
really tells whether there is a DSA tagging protocol set-up and that is
what NIC drivers (e.g: bcmsysport) would care about.
Thanks!
--
Florian
