[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <541D0EBC.6010103@gmail.com>
Date: Fri, 19 Sep 2014 22:21:00 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
CC: davem@...emloft.net, nhorman@...driver.com, andy@...yhouse.net,
tgraf@...g.ch, dborkman@...hat.com, ogerlitz@...lanox.com,
jesse@...ira.com, pshelar@...ira.com, azhou@...ira.com,
ben@...adent.org.uk, stephen@...workplumber.org,
jeffrey.t.kirsher@...el.com, vyasevic@...hat.com,
xiyou.wangcong@...il.com, john.r.fastabend@...el.com,
edumazet@...gle.com, jhs@...atatu.com, sfeldma@...ulusnetworks.com,
roopa@...ulusnetworks.com, linville@...driver.com,
dev@...nvswitch.org, jasowang@...hat.com, ebiederm@...ssion.com,
nicolas.dichtel@...nd.com, ryazanov.s.a@...il.com,
buytenh@...tstofly.org, aviadr@...lanox.com, nbd@...nwrt.org,
alexei.starovoitov@...il.com, Neil.Jerram@...aswitch.com,
ronye@...lanox.com, simon.horman@...ronome.com,
alexander.h.duyck@...el.com
Subject: Re: [patch net-next v2 5/9] net: introduce dummy switch
On 09/19/14 06:49, Jiri Pirko wrote:
> Dummy switch implementation using switchdev interface
This really looks like a DSA driver that has 0 ports, and is not
attached to an useful network interface, and which is registering its
own set of rtnl operations for a purpose that is unclear to me.
I think registering these rtnl ops is misleading as it leads to a false
idea that this is allowed, and that people are actually encouraged to do
that for custom switch drivers, and this completely defeats the purpose
of coming up with a generic API.
If we are to go that route anyway, I really prefer the way Felix did it
in swconfig, and the fake switch driver did do something useful being
attached to the loopback interface:
http://lists.openwall.net/netdev/2013/10/22/103
http://patchwork.ozlabs.org/patch/285478/
>
> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
> ---
> drivers/net/Kconfig | 7 +++
> drivers/net/Makefile | 1 +
> drivers/net/dummyswitch.c | 130 +++++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/if_link.h | 9 +++
> 4 files changed, 147 insertions(+)
> create mode 100644 drivers/net/dummyswitch.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index c6f6f69..7822c74 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -71,6 +71,13 @@ config DUMMY
> To compile this driver as a module, choose M here: the module
> will be called dummy.
>
> +config NET_DUMMY_SWITCH
> + tristate "Dummy switch net driver support"
> + depends on NET_SWITCHDEV
> + ---help---
> + To compile this driver as a module, choose M here: the module
> + will be called dummyswitch.
> +
> config EQUALIZER
> tristate "EQL (serial line load balancing) support"
> ---help---
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 61aefdd..3c835ba 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -7,6 +7,7 @@
> #
> obj-$(CONFIG_BONDING) += bonding/
> obj-$(CONFIG_DUMMY) += dummy.o
> +obj-$(CONFIG_NET_DUMMY_SWITCH) += dummyswitch.o
> obj-$(CONFIG_EQUALIZER) += eql.o
> obj-$(CONFIG_IFB) += ifb.o
> obj-$(CONFIG_MACVLAN) += macvlan.o
> diff --git a/drivers/net/dummyswitch.c b/drivers/net/dummyswitch.c
> new file mode 100644
> index 0000000..e7a48f4
> --- /dev/null
> +++ b/drivers/net/dummyswitch.c
> @@ -0,0 +1,130 @@
> +/*
> + * drivers/net/dummyswitch.c - Dummy switch device
> + * Copyright (c) 2014 Jiri Pirko <jiri@...nulli.us>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <net/rtnetlink.h>
> +
> +struct dummyswport_priv {
> + struct netdev_phys_item_id psid;
> +};
> +
> +static netdev_tx_t dummyswport_start_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> +}
> +
> +static int dummyswport_swdev_id_get(struct net_device *dev,
> + struct netdev_phys_item_id *psid)
> +{
> + struct dummyswport_priv *dsp = netdev_priv(dev);
> +
> + memcpy(psid, &dsp->psid, sizeof(*psid));
> + return 0;
> +}
> +
> +static int dummyswport_change_carrier(struct net_device *dev, bool new_carrier)
> +{
> + if (new_carrier)
> + netif_carrier_on(dev);
> + else
> + netif_carrier_off(dev);
> + return 0;
> +}
> +
> +static const struct net_device_ops dummyswport_netdev_ops = {
> + .ndo_start_xmit = dummyswport_start_xmit,
> + .ndo_swdev_id_get = dummyswport_swdev_id_get,
> + .ndo_change_carrier = dummyswport_change_carrier,
> +};
> +
> +static void dummyswport_setup(struct net_device *dev)
> +{
> + ether_setup(dev);
> +
> + /* Initialize the device structure. */
> + dev->netdev_ops = &dummyswport_netdev_ops;
> + dev->destructor = free_netdev;
> +
> + /* Fill in device structure with ethernet-generic values. */
> + dev->tx_queue_len = 0;
> + dev->flags |= IFF_NOARP;
> + dev->flags &= ~IFF_MULTICAST;
> + dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> + dev->features |= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO;
> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
> + eth_hw_addr_random(dev);
> +}
> +
> +static int dummyswport_validate(struct nlattr *tb[], struct nlattr *data[])
> +{
> + if (tb[IFLA_ADDRESS])
> + return -EINVAL;
> + if (!data || !data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID])
> + return -EINVAL;
> + return 0;
> +}
> +
> +static int dummyswport_newlink(struct net *src_net, struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[])
> +{
> + struct dummyswport_priv *dsp = netdev_priv(dev);
> + int err;
> +
> + dsp->psid.id_len = nla_len(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]);
> + memcpy(dsp->psid.id, nla_data(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]),
> + dsp->psid.id_len);
> +
> + err = register_netdevice(dev);
> + if (err)
> + return err;
> +
> + netif_carrier_on(dev);
> +
> + return 0;
> +}
> +
> +static const struct nla_policy dummyswport_policy[IFLA_DUMMYSWPORT_MAX + 1] = {
> + [IFLA_DUMMYSWPORT_PHYS_SWITCH_ID] = { .type = NLA_BINARY,
> + .len = MAX_PHYS_ITEM_ID_LEN },
> +};
> +
> +static struct rtnl_link_ops dummyswport_link_ops __read_mostly = {
> + .kind = "dummyswport",
> + .priv_size = sizeof(struct dummyswport_priv),
> + .setup = dummyswport_setup,
> + .validate = dummyswport_validate,
> + .newlink = dummyswport_newlink,
> + .policy = dummyswport_policy,
> + .maxtype = IFLA_DUMMYSWPORT_MAX,
> +};
> +
> +static int __init dummysw_module_init(void)
> +{
> + return rtnl_link_register(&dummyswport_link_ops);
> +}
> +
> +static void __exit dummysw_module_exit(void)
> +{
> + rtnl_link_unregister(&dummyswport_link_ops);
> +}
> +
> +module_init(dummysw_module_init);
> +module_exit(dummysw_module_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Jiri Pirko <jiri@...nulli.us>");
> +MODULE_DESCRIPTION("Dummy switch device");
> +MODULE_ALIAS_RTNL_LINK("dummyswport");
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index c5ca3b9..bd24d69 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -574,4 +574,13 @@ enum {
>
> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)
>
> +/* DUMMYSWPORT section */
> +enum {
> + IFLA_DUMMYSWPORT_UNSPEC,
> + IFLA_DUMMYSWPORT_PHYS_SWITCH_ID,
> + __IFLA_DUMMYSWPORT_MAX,
> +};
> +
> +#define IFLA_DUMMYSWPORT_MAX (__IFLA_DUMMYSWPORT_MAX - 1)
> +
> #endif /* _UAPI_LINUX_IF_LINK_H */
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists