On Wed, Apr 05, 2017 at 11:20:21AM +0200, Juergen Borleis wrote:
> To define the outgoing port and to discover the incoming port a regular
> VLAN tag is used by the LAN9303. But its VID meaning is 'special'.
> 
> This tag handler/filter depends on some hardware features which must be
> enabled in the device to provide and make use of this special VLAN tag
> to control the destination and the source of an ethernet packet.
> 
> Signed-off-by: Juergen Borleis <j...@pengutronix.de>
> ---
>  include/net/dsa.h     |   1 +
>  net/dsa/Kconfig       |   3 +
>  net/dsa/Makefile      |   1 +
>  net/dsa/dsa.c         |   3 +
>  net/dsa/dsa_priv.h    |   3 +
>  net/dsa/tag_lan9303.c | 155 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 166 insertions(+)
>  create mode 100644 net/dsa/tag_lan9303.c
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 4e13e695f0251..4fb1f2b086b05 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -31,6 +31,7 @@ enum dsa_tag_protocol {
>       DSA_TAG_PROTO_EDSA,
>       DSA_TAG_PROTO_BRCM,
>       DSA_TAG_PROTO_QCA,
> +     DSA_TAG_PROTO_LAN9303,
>       DSA_TAG_LAST,           /* MUST BE LAST */
>  };
>  
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 9649238eef404..22c8bd69ff71c 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -31,4 +31,7 @@ config NET_DSA_TAG_TRAILER
>  config NET_DSA_TAG_QCA
>       bool
>  
> +config NET_DSA_TAG_LAN9303
> +     bool
> +
>  endif
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 31d343796251d..aafc74f2cb193 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -8,3 +8,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
>  dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
> +dsa_core-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index b6d4f6a23f06c..f93f78de23af3 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -53,6 +53,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = 
> {
>  #ifdef CONFIG_NET_DSA_TAG_QCA
>       [DSA_TAG_PROTO_QCA] = &qca_netdev_ops,
>  #endif
> +#ifdef CONFIG_NET_DSA_TAG_LAN9303
> +     [DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
> +#endif
>       [DSA_TAG_PROTO_NONE] = &none_ops,
>  };
>  
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 0706a511244e9..a54cfc8aefa83 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -85,4 +85,7 @@ extern const struct dsa_device_ops brcm_netdev_ops;
>  /* tag_qca.c */
>  extern const struct dsa_device_ops qca_netdev_ops;
>  
> +/* tag_lan9303.c */
> +extern const struct dsa_device_ops lan9303_netdev_ops;
> +
>  #endif
> diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
> new file mode 100644
> index 0000000000000..ad04c6d447f77
> --- /dev/null
> +++ b/net/dsa/tag_lan9303.c
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (C) 2017 Pengutronix, Juergen Borleis <j...@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/etherdevice.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include "dsa_priv.h"
> +
> +/* To define the outgoing port and to discover the incoming port a regular
> + * VLAN tag is used by the LAN9303. But its VID meaning is 'special':
> + *
> + *       Dest MAC       Src MAC        TAG    Type
> + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 | 1 2 |...
> + *                                |<------->|
> + * TAG:
> + *    |<------------->|
> + *    |  1  2 | 3  4  |
> + *      TPID    VID
> + *     0x8100
> + *
> + * VID bit 3 indicates a request for an ALR lookup.
> + *
> + * If VID bit 3 is zero, then bits 0 and 1 specify the destination port
> + * (0, 1, 2) or broadcast (3) or the source port (1, 2).
> + *
> + * VID bit 4 is used to specify if the STP port state should be overridden.
> + * Required when no forwarding between the external ports should happen.
> + */

Hi Juergen

Nice comment, thanks.

> +
> +#define LAN9303_TAG_LEN 4
> +
> +static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device 
> *dev)
> +{

...

> +     /* make room between MACs and Ether-Type */
> +     memmove(skb->data, skb->data + LAN9303_TAG_LEN, 2 * ETH_ALEN);
> +
> +     lan9303_tag = (u16 *)(skb->data + 2 * ETH_ALEN);
> +     lan9303_tag[0] = htons(ETH_P_8021Q);
> +     lan9303_tag[1] = htons(p->dp->index | BIT(4));

> +static int lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
> +                    struct packet_type *pt, struct net_device *orig_dev)
> +{
> +     u16 *lan9303_tag;
> +     struct dsa_switch_tree *dst = dev->dsa_ptr;
> +     struct dsa_switch *ds = dst->ds[0];
> +     unsigned int source_port;
> +
> +     if (unlikely(!dst)) {
> +             dev_warn_ratelimited(&dev->dev, "Dropping packet, due to 
> missing switch tree device\n");
> +             goto out_drop;
> +     }

By the time you get here, you have already dereferenced dst, in order
to get ds. So this is pointless.

> +     lan9303_tag = (u16 *)(skb->data - 2);
> +
> +     if (lan9303_tag[0] != htons(0x8100)) {
> +             dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid 
> VLAN marker\n");
> +             goto out_drop;
> +     }

In the transmit function, you use ETH_P_8021Q, please do so here as
well.

> +     source_port = ntohs(lan9303_tag[1]) & 0x3;
> +
> +     if (source_port >= DSA_MAX_PORTS) {
> +             dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid 
> source port\n");
> +             goto out_drop;
> +     }

You can be more specific here. For this hardware, > 3 is invalid and
should be dropped. If we later add other chips which have more ports,
we can relax this check then.

> +     /* remove the special VLAN tag between the MAC addresses
> +      * and the current ethertype field.
> +      */
> +     skb_pull_rcsum(skb, 2 + 2);
> +     memmove(skb->data - ETH_HLEN, skb->data - (ETH_HLEN + LAN9303_TAG_LEN),
> +             2 * ETH_ALEN);
> +     skb_push(skb, ETH_HLEN);

Do you need to do anything with the checksum here? Other tagging
protocols do.

          Andrew

Reply via email to