[OpenWrt-Devel] [PATCH 3/3] b53: create slave devices for ports

Florian Fainelli f.fainelli at gmail.com
Thu Feb 26 13:24:24 EST 2015


On 25/02/15 07:24, Alexandru Ardelean wrote:
> Feature implemented and tested on BCM53128.
> 
> Slave devices logic copied from the Linux kernel from Marvell's DSA
> driver ( linux/net/dsa/ ).
> Also the logic for the Broadcom tag processing has been copied from there.

There are different efforts here going on, and I would like to at least
3 different people (you, Rafal and myself) can converge to an identical
solution that fits everybody here.

What net-next supports today is:

- broadcom tags in net/dsa/tag_brcm.c, 4-bytes format, identical to
yours AFAICT
- HW bridging support for bcm_sf2 (roboswitch successor)

What's missing:
- adding VLAN configuration, which is what Rafal has been doing using
here: http://thread.gmane.org/gmane.linux.network/351503

There are a number of things that I want to rework in DSA such that we
can almost immediately leverage OpenWrt's switch drivers, where the
entry point is a phy_driver, and have them register as switches (DSA or
something wider) eventually [1], such that DSA handles the slave devices
creation, and also handles the transmission/reception of Broadcom tags
for us. This is work in progress, but I expect the patches would be
ready by the end of this week.

[1]: http://www.spinics.net/lists/netdev/msg295942.html

> 
> OpenWRT's eth_mangle_rx/tx() patch/code is being used to tap into
> the packets to/from the ethernet chip since it's convenient.
> 
> This code will create lanX (X = 1..B53_N_PORTS) devices.
> All traffic from the ethX device will be forwarded the proper lanX device.
> So, sw_port0_traffic == lan1_traffic and so on.
> 
> The slave devices logic has been put into it's own file.
> Should this logic be desired to be extended to swconfig or other
> switch chips, it should be convenient to just move the slave.c/h files.
> 
> Note: if enable_vlan == 1, be sure to configure VLAN per lanX device
>       in '/etc/config/network'
> 
> Signed-off-by: Alexandru Ardelean <ardeleanalex at gmail.com>
> ---
>  .../generic/files/drivers/net/phy/b53/Makefile     |   2 +-
>  .../generic/files/drivers/net/phy/b53/b53_common.c |   3 +
>  .../generic/files/drivers/net/phy/b53/b53_hdr.c    | 114 +++++++++++-
>  .../generic/files/drivers/net/phy/b53/b53_priv.h   |   2 +
>  .../generic/files/drivers/net/phy/b53/slave.c      | 196 +++++++++++++++++++++
>  .../generic/files/drivers/net/phy/b53/slave.h      |  38 ++++
>  6 files changed, 352 insertions(+), 3 deletions(-)
>  create mode 100644 target/linux/generic/files/drivers/net/phy/b53/slave.c
>  create mode 100644 target/linux/generic/files/drivers/net/phy/b53/slave.h
> 
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/Makefile b/target/linux/generic/files/drivers/net/phy/b53/Makefile
> index 6c809f9..c74f82b 100644
> --- a/target/linux/generic/files/drivers/net/phy/b53/Makefile
> +++ b/target/linux/generic/files/drivers/net/phy/b53/Makefile
> @@ -1,5 +1,5 @@
>  obj-$(CONFIG_B53)		+= b53_common.o
> -obj-$(CONFIG_B53_HDR)		+= b53_hdr.o
> +obj-$(CONFIG_B53_HDR)		+= b53_hdr.o slave.o
>  
>  obj-$(CONFIG_B53_PHY_FIXUP)	+= b53_phy_fixup.o
>  
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> index 9459b22..3da9efe 100644
> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_common.c
> @@ -1377,6 +1377,9 @@ static int b53_global_reset_switch(struct switch_dev *dev)
>  	priv->enable_management = 0;
>  #ifdef CONFIG_B53_HDR
>  	priv->enable_brcm_hdr = 0;
> +	/* Call this function before the memset on the priv->ports, 
> +	 * otherwise we may leak devices */
> +	b53_unregister_netdevs(priv);
>  #endif
>  
>  	memset(priv->vlans, 0, sizeof(priv->vlans) * dev->vlans);
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_hdr.c b/target/linux/generic/files/drivers/net/phy/b53/b53_hdr.c
> index 2a562a9..8fa7929 100644
> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_hdr.c
> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_hdr.c
> @@ -21,14 +21,32 @@
>  
>  #include "b53_regs.h"
>  #include "b53_priv.h"
> +#include "slave.h"
>  
>  /* This tag length is 4 bytes, older ones were 6 bytes, we do not
>   * handle them
>   */
>  #define BRCM_HDR_LEN    4
>  #define MIN_FRAME_LEN   64
> +#define BRCM_IG_DSTMAP2_MASK 1
> +#define BRCM_IG_DSTMAP1_MASK 0xff
> +#define BRCM_EG_PID_MASK     0x1f
>  
> -static struct sk_buff *b53_mangle_tx(struct net_device *dev, struct sk_buff *skb)
> +/* Ingress and egress opcodes */
> +#define BRCM_OPCODE_SHIFT   5
> +
> +static inline void b53_brcm_tag_set(struct sk_buff *skb, int port_idx)
> +{
> +	u8 *brcm_tag = skb->data + 2 * ETH_ALEN;
> +
> +	brcm_tag[0] = (1 << BRCM_OPCODE_SHIFT);
> +	brcm_tag[1] = 0;
> +	brcm_tag[2] = (port_idx == 8) ? BRCM_IG_DSTMAP2_MASK : 0;
> +	brcm_tag[3] = (1 << port_idx) & BRCM_IG_DSTMAP1_MASK;
> +}
> +
> +static struct sk_buff *b53_mangle_tx_port(struct net_device *dev,
> +			struct sk_buff *skb, struct slave_priv* p)
>  {
>  	if (unlikely(skb_headroom(skb) < BRCM_HDR_LEN)) {
>  		if (pskb_expand_head(skb, BRCM_HDR_LEN, 0, GFP_ATOMIC) < 0)
> @@ -40,7 +58,20 @@ static struct sk_buff *b53_mangle_tx(struct net_device *dev, struct sk_buff *skb
>  	memmove(skb->data, skb->data + BRCM_HDR_LEN, 2 * ETH_ALEN);
>  
>  	/* Build the tag after the MAC Source Address */
> -	memset(skb->data + 2 * ETH_ALEN, 0, BRCM_HDR_LEN);
> +	if (!p) {
> +		memset(skb->data + 2 * ETH_ALEN, 0, BRCM_HDR_LEN);
> +	} else {
> +		/* Register some TX stats for this device before
> +		 * passing the skb to the parent device */
> +		dev->stats.tx_packets++;
> +		dev->stats.tx_bytes += skb->len;
> +
> +		b53_brcm_tag_set(skb, p->port_idx);
> +
> +		skb->dev = p->parent_dev;
> +		skb->protocol = htons(ETH_P_DSA);
> +	}
> +
>  #ifdef CONFIG_B53_HDR_TX_SW_PADDING
>  	/* FIXME: we're doing some padding here for runt ( < 64 bytes) packets;
>  	 *        some drivers/hw refuse to add hw padding for us after we add
> @@ -64,12 +95,88 @@ out_err:
>  	return NULL;
>  }
>  
> +static struct sk_buff *b53_mangle_tx(struct net_device *dev, struct sk_buff *skb)
> +{
> +	/* The b53_mangle_tx() function can get called twice now that there are
> +	 * slave devices: once for the lanX device, and one for the ethY device
> +	 * which means that the Broadcom Header would get added twice.
> +	 * Which is why the packet type has been marked by the slave lanX device
> +	 * to tell us that we've tagged this packet already. */
> +	if (unlikely(skb->protocol == htons(ETH_P_DSA)))
> +		return skb;
> +	return b53_mangle_tx_port(dev, skb, NULL);
> +}
> +
>  static void b53_mangle_rx(struct net_device *dev, struct sk_buff *skb)
>  {
> +	struct b53_device *p = dev->phy_ptr;
> +	u8 *brcm_tag;
> +	u8 source_port;
> +
> +	/* We're only interested in the 4th byte of the Broadcom Header right now */
> +	brcm_tag = skb->data + (2 * ETH_ALEN) + 3;
> +	source_port = *brcm_tag & BRCM_EG_PID_MASK;
> +
>  	skb_pull(skb, BRCM_HDR_LEN);
>  	memmove(skb->data,
>  		skb->data - BRCM_HDR_LEN,
>  		2 * ETH_ALEN);
> +
> +	if ((source_port < B53_N_PORTS) && (p->ports[source_port].port_dev)) {
> +		skb->dev = p->ports[source_port].port_dev;
> +		skb->dev->stats.rx_packets++;
> +		skb->dev->stats.rx_bytes += skb->len;
> +	}
> +}
> +
> +static void b53_register_netdevs(struct b53_device *dev)
> +{
> +	int i;
> +	bool unlock = false;
> +
> +	if (!rtnl_is_locked()) {
> +		rtnl_lock();
> +		unlock = true;
> +	}
> +	b53_for_each_port(dev, i) {
> +		if (is_cpu_port(dev, i))
> +			continue;
> +		if (!(dev->enabled_ports & BIT(i))) {
> +			if (dev->ports[i].port_dev) {
> +				unregister_netdevice(dev->ports[i].port_dev);
> +				dev->ports[i].port_dev = NULL;
> +			}
> +			continue;
> +		}
> +		/* Check if devices already exist for these devices */
> +		if (!dev->ports[i].port_dev && 
> +		    !(dev->ports[i].port_dev = slave_create(dev->eth_dev, i, b53_mangle_tx_port))) {
> +			pr_warn("%s: can't create slave device for port %d\n",
> +			        dev->sw_dev.name, i);
> +			continue;
> +		}
> +	}
> +	if (unlock)
> +		rtnl_unlock();
> +}
> +
> +void b53_unregister_netdevs(struct b53_device *dev)
> +{
> +	int i;
> +	bool unlock = false;
> +
> +	if (!rtnl_is_locked()) {
> +		rtnl_lock();
> +		unlock = true;
> +	}
> +	b53_for_each_port(dev, i) {
> +		if (!dev->ports[i].port_dev)
> +			continue;
> +		unregister_netdevice(dev->ports[i].port_dev);
> +		dev->ports[i].port_dev = NULL;
> +	}
> +	if (unlock)
> +		rtnl_unlock();
>  }
>  
>  void b53_enable_brcm_hdr(struct b53_device *dev)
> @@ -86,6 +193,7 @@ void b53_enable_brcm_hdr(struct b53_device *dev)
>  	}
>  	brcm_hdr_ctrl &= ~B53_BRCM_HDR_EN;
>  	if (!dev->eth_dev) {
> +		b53_unregister_netdevs(dev);
>  		goto out;
>  	}
>  
> @@ -93,6 +201,8 @@ void b53_enable_brcm_hdr(struct b53_device *dev)
>  	if (!dev->enable_management)
>  		goto out;
>  
> +	pr_info("%s: registering lan devices\n", dev->sw_dev.name);
> +	b53_register_netdevs(dev);
>  	dev->eth_dev->phy_ptr = dev;
>  	dev->eth_dev->priv_flags |= IFF_NO_IP_ALIGN;
>  	dev->eth_dev->eth_mangle_rx = b53_mangle_rx;
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/b53_priv.h b/target/linux/generic/files/drivers/net/phy/b53/b53_priv.h
> index f487bf2..167e042 100644
> --- a/target/linux/generic/files/drivers/net/phy/b53/b53_priv.h
> +++ b/target/linux/generic/files/drivers/net/phy/b53/b53_priv.h
> @@ -359,8 +359,10 @@ int b53_global_get_brcm_hdr(struct switch_dev *dev,
>  int b53_global_set_brcm_hdr(struct switch_dev *dev,
>  		const struct switch_attr *attr,
>  		struct switch_val *val);
> +void b53_unregister_netdevs(struct b53_device *dev);
>  #else
>  #define b53_enable_brcm_hdr(x)
> +#define b53_unregister_netdevs(x)
>  #endif
>  
>  #ifdef CONFIG_BCM47XX
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/slave.c b/target/linux/generic/files/drivers/net/phy/b53/slave.c
> new file mode 100644
> index 0000000..b8cd2dc
> --- /dev/null
> +++ b/target/linux/generic/files/drivers/net/phy/b53/slave.c
> @@ -0,0 +1,196 @@
> +/*
> + * Slave devices logic.
> + * Adapted/copied from the Linux kernel DSA driver (net/dsa).
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <linux/rtnetlink.h>
> +#include <linux/etherdevice.h>
> +
> +#include "slave.h"
> +
> +static int slave_init(struct net_device *dev)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +	dev->iflink = p->parent_dev->ifindex;
> +	return 0;
> +}
> +
> +static int slave_open(struct net_device *dev)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +	struct net_device *parent = p->parent_dev;
> +	int err;
> +
> +	if (!(parent->flags & IFF_UP))
> +		return -ENETDOWN;
> +
> +	if (!ether_addr_equal(dev->dev_addr, parent->dev_addr)) {
> +		err = dev_uc_add(parent, dev->dev_addr);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +	if (dev->flags & IFF_ALLMULTI) {
> +		err = dev_set_allmulti(parent, 1);
> +		if (err < 0)
> +			goto del_unicast;
> +	}
> +	if (dev->flags & IFF_PROMISC) {
> +		err = dev_set_promiscuity(parent, 1);
> +		if (err < 0)
> +			goto clear_allmulti;
> +	}
> +
> +	return 0;
> +
> +clear_allmulti:
> +	if (dev->flags & IFF_ALLMULTI)
> +		dev_set_allmulti(parent, -1);
> +del_unicast:
> +	if (!ether_addr_equal(dev->dev_addr, parent->dev_addr))
> +		dev_uc_del(parent, dev->dev_addr);
> +out:
> +	return err;
> +}
> +
> +static int slave_close(struct net_device *dev)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +	struct net_device *parent = p->parent_dev;
> +
> +	dev_mc_unsync(parent, dev);
> +	dev_uc_unsync(parent, dev);
> +	if (dev->flags & IFF_ALLMULTI)
> +		dev_set_allmulti(parent, -1);
> +	if (dev->flags & IFF_PROMISC)
> +		dev_set_promiscuity(parent, -1);
> +
> +	if (!ether_addr_equal(dev->dev_addr, parent->dev_addr))
> +		dev_uc_del(parent, dev->dev_addr);
> +
> +	return 0;
> +}
> +
> +static void slave_change_rx_flags(struct net_device *dev, int change)
> +{
> +    struct slave_priv *p = netdev_priv(dev);
> +	struct net_device *parent = p->parent_dev;
> +
> +	if (change & IFF_ALLMULTI)
> +		dev_set_allmulti(parent, dev->flags & IFF_ALLMULTI ? 1 : -1);
> +	if (change & IFF_PROMISC)
> +		dev_set_promiscuity(parent, dev->flags & IFF_PROMISC ? 1 : -1);
> +}
> +
> +static void slave_set_rx_mode(struct net_device *dev)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +	struct net_device *parent = p->parent_dev;
> +
> +	dev_mc_sync(parent, dev);
> +	dev_uc_sync(parent, dev);
> +}
> +
> +static int slave_set_mac_address(struct net_device *dev, void *a)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +	struct net_device *parent = p->parent_dev;
> +	struct sockaddr *addr = a;
> +	int err;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	if (!(dev->flags & IFF_UP))
> +		goto out;
> +
> +	if (!ether_addr_equal(addr->sa_data, parent->dev_addr)) {
> +		err = dev_uc_add(parent, addr->sa_data);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	if (!ether_addr_equal(dev->dev_addr, parent->dev_addr))
> +		dev_uc_del(parent, dev->dev_addr);
> +
> +out:
> +	memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
> +
> +	return 0;
> +}
> +
> +static netdev_tx_t slave_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct slave_priv *p = netdev_priv(dev);
> +
> +	skb->dev = p->parent_dev;
> +	skb = p->eth_mangle_tx_port(dev, skb, p);
> +	dev_queue_xmit(skb);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops slave_netdev_ops = {
> +	.ndo_init		= slave_init,
> +	.ndo_open		= slave_open,
> +	.ndo_stop		= slave_close,
> +	.ndo_start_xmit		= slave_xmit,
> +	.ndo_change_rx_flags	= slave_change_rx_flags,
> +	.ndo_set_rx_mode	= slave_set_rx_mode,
> +	.ndo_set_mac_address	= slave_set_mac_address,
> +};
> +
> +struct net_device *slave_create(struct net_device *parent, int port,
> +		eth_mangle_tx_port_t eth_mangle_tx_port)
> +{
> +	/* Parent dev should not be null according to the code path below */
> +	struct net_device *slave_dev = NULL;
> +	struct slave_priv *p;
> +	char port_name[] = "lan1000";
> +
> +	snprintf(port_name, sizeof(port_name), "lan%d", (port + 1));
> +	slave_dev = alloc_netdev(sizeof(struct slave_priv),
> +	            port_name, ether_setup);
> +	if (!slave_dev) {
> +		pr_err("Failed to allocate memory for slave device");
> +		goto out;
> +	}
> +
> +	slave_dev->features = parent->vlan_features;
> +	eth_hw_addr_inherit(slave_dev, parent);
> +	slave_dev->tx_queue_len = 0;
> +	slave_dev->netdev_ops = &slave_netdev_ops;
> +
> +	SET_NETDEV_DEV(slave_dev, &parent->dev);
> +	slave_dev->vlan_features = parent->vlan_features;
> +
> +	p = netdev_priv(slave_dev);
> +	p->parent_dev = parent;
> +	p->port_dev   = slave_dev;
> +	p->port_idx   = port;
> +	p->eth_mangle_tx_port = eth_mangle_tx_port;
> +
> +	if (register_netdevice(slave_dev)) {
> +		free_netdev(slave_dev);
> +		slave_dev = NULL;
> +		goto out;
> +	}
> +
> +	netif_carrier_on(slave_dev);
> +
> +out:
> +	return slave_dev;
> +}
> +
> diff --git a/target/linux/generic/files/drivers/net/phy/b53/slave.h b/target/linux/generic/files/drivers/net/phy/b53/slave.h
> new file mode 100644
> index 0000000..3cfe7c7
> --- /dev/null
> +++ b/target/linux/generic/files/drivers/net/phy/b53/slave.h
> @@ -0,0 +1,38 @@
> +/*
> + * Slave devices logic.
> + * Adapted/copied from the Linux kernel DSA driver (net/dsa).
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#ifndef __SLAVE_H__
> +#define __SLAVE_H__
> +
> +#include <linux/etherdevice.h>
> +
> +struct slave_priv;
> +typedef struct sk_buff *(*eth_mangle_tx_port_t)(struct net_device *dev,
> +		struct sk_buff *skb, struct slave_priv* p);
> +
> +struct slave_priv {
> +	struct net_device *parent_dev;
> +	struct net_device *port_dev;
> +	eth_mangle_tx_port_t eth_mangle_tx_port;
> +	int port_idx;
> +};
> +
> +/* For the moment, the rtnl_lock() needs to be called by the caller */
> +struct net_device *slave_create(struct net_device *parent, int port,
> +		eth_mangle_tx_port_t eth_mangle_tx_port);
> +
> +#endif /* __SLAVE_H__ */
> 


-- 
Florian
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list