| 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
| ||
|
Message-ID: <20100408083710.2b61ee44@nehalam>
Date: Thu, 8 Apr 2010 08:37:10 -0700
From: Stephen Hemminger <shemminger@...ux-foundation.org>
To: Amerigo Wang <amwang@...hat.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
bridge@...ts.linux-foundation.org,
Andy Gospodarek <gospo@...hat.com>,
Neil Horman <nhorman@...driver.com>,
Jeff Moyer <jmoyer@...hat.com>, Matt Mackall <mpm@...enic.com>,
bonding-devel@...ts.sourceforge.net,
Jay Vosburgh <fubar@...ibm.com>,
David Miller <davem@...emloft.net>
Subject: Re: [v3 Patch 2/3] bridge: make bridge support netpoll
On Thu, 8 Apr 2010 02:18:58 -0400
Amerigo Wang <amwang@...hat.com> wrote:
>
> Based on the previous patch, make bridge support netpoll by:
>
> 1) implement the 2 methods to support netpoll for bridge;
>
> 2) modify netpoll during forwarding packets via bridge;
>
> 3) disable netpoll support of bridge when a netpoll-unabled device
> is added to bridge;
>
> 4) enable netpoll support when all underlying devices support netpoll.
>
> Cc: David Miller <davem@...emloft.net>
> Cc: Neil Horman <nhorman@...driver.com>
> Cc: Stephen Hemminger <shemminger@...ux-foundation.org>
> Cc: Matt Mackall <mpm@...enic.com>
> Signed-off-by: WANG Cong <amwang@...hat.com>
>
> ---
>
> Index: linux-2.6/net/bridge/br_device.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_device.c
> +++ linux-2.6/net/bridge/br_device.c
> @@ -13,8 +13,10 @@
>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/etherdevice.h>
> #include <linux/ethtool.h>
> +#include <linux/list.h>
>
> #include <asm/uaccess.h>
> #include "br_private.h"
> @@ -162,6 +164,59 @@ static int br_set_tx_csum(struct net_dev
> return 0;
> }
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +bool br_devices_support_netpoll(struct net_bridge *br)
> +{
> + struct net_bridge_port *p;
> + bool ret = true;
> + int count = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&br->lock, flags);
> + list_for_each_entry(p, &br->port_list, list) {
> + count++;
> + if (p->dev->priv_flags & IFF_DISABLE_NETPOLL
> + || !p->dev->netdev_ops->ndo_poll_controller)
> + ret = false;
> + }
> + spin_unlock_irqrestore(&br->lock, flags);
> + return count != 0 && ret;
> +}
> +
> +static void br_poll_controller(struct net_device *br_dev)
> +{
> + struct netpoll *np = br_dev->npinfo->netpoll;
> +
> + if (np->real_dev != br_dev)
> + netpoll_poll_dev(np->real_dev);
> +}
> +
> +void br_netpoll_cleanup(struct net_device *br_dev)
> +{
> + struct net_bridge *br = netdev_priv(br_dev);
> + struct net_bridge_port *p, *n;
> + const struct net_device_ops *ops;
> +
> + br->dev->npinfo = NULL;
> + list_for_each_entry_safe(p, n, &br->port_list, list) {
> + if (p->dev) {
> + ops = p->dev->netdev_ops;
> + if (ops->ndo_netpoll_cleanup)
> + ops->ndo_netpoll_cleanup(p->dev);
> + else
> + p->dev->npinfo = NULL;
> + }
> + }
> +}
> +
> +#else
> +
> +void br_netpoll_cleanup(struct net_device *br_dev)
> +{
> +}
> +
> +#endif
Could you use more stub functions to eliminate #ifdef's in code.
> static const struct ethtool_ops br_ethtool_ops = {
> .get_drvinfo = br_getinfo,
> .get_link = ethtool_op_get_link,
> @@ -184,6 +239,10 @@ static const struct net_device_ops br_ne
> .ndo_set_multicast_list = br_dev_set_multicast_list,
> .ndo_change_mtu = br_change_mtu,
> .ndo_do_ioctl = br_dev_ioctl,
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + .ndo_netpoll_cleanup = br_netpoll_cleanup,
> + .ndo_poll_controller = br_poll_controller,
> +#endif
> };
>
> void br_dev_setup(struct net_device *dev)
> Index: linux-2.6/net/bridge/br_forward.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_forward.c
> +++ linux-2.6/net/bridge/br_forward.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/skbuff.h>
> #include <linux/if_vlan.h>
> #include <linux/netfilter_bridge.h>
> @@ -50,7 +51,13 @@ int br_dev_queue_push_xmit(struct sk_buf
> else {
> skb_push(skb, ETH_HLEN);
>
> - dev_queue_xmit(skb);
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (skb->dev->priv_flags & IFF_IN_NETPOLL) {
> + netpoll_send_skb(skb->dev->npinfo->netpoll, skb);
> + skb->dev->priv_flags &= ~IFF_IN_NETPOLL;
> + } else
> +#endif
There is no protection on dev->priv_flags for SMP access.
It would better bit value in dev->state if you are using it as control flag.
Then you could use
if (unlikely(test_and_clear_bit(__IN_NETPOLL, &skb->dev->state)))
netpoll_send_skb(...)
> + dev_queue_xmit(skb);
> }
> }
>
> @@ -66,9 +73,23 @@ int br_forward_finish(struct sk_buff *sk
>
> static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
> {
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + struct net_bridge *br = to->br;
> + if (br->dev->priv_flags & IFF_IN_NETPOLL) {
> + struct netpoll *np;
> + to->dev->npinfo = skb->dev->npinfo;
> + np = skb->dev->npinfo->netpoll;
> + np->real_dev = np->dev = to->dev;
> + to->dev->priv_flags |= IFF_IN_NETPOLL;
> + }
> +#endif
This is n hot path, so use unlikely()
> skb->dev = to->dev;
> NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
> br_forward_finish);
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (skb->dev->npinfo)
> + skb->dev->npinfo->netpoll->dev = br->dev;
> +#endif
> }
>
> static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
> Index: linux-2.6/net/bridge/br_if.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_if.c
> +++ linux-2.6/net/bridge/br_if.c
> @@ -13,6 +13,7 @@
>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/ethtool.h>
> #include <linux/if_arp.h>
> #include <linux/module.h>
> @@ -153,6 +154,14 @@ static void del_nbp(struct net_bridge_po
> kobject_uevent(&p->kobj, KOBJ_REMOVE);
> kobject_del(&p->kobj);
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (br_devices_support_netpoll(br))
> + br->dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
> + if (dev->netdev_ops->ndo_netpoll_cleanup)
> + dev->netdev_ops->ndo_netpoll_cleanup(dev);
> + else
> + dev->npinfo = NULL;
> +#endif
> call_rcu(&p->rcu, destroy_nbp_rcu);
> }
>
> @@ -165,6 +174,8 @@ static void del_br(struct net_bridge *br
> del_nbp(p);
> }
>
> + br_netpoll_cleanup(br->dev);
> +
> del_timer_sync(&br->gc_timer);
>
> br_sysfs_delbr(br->dev);
> @@ -438,6 +449,20 @@ int br_add_if(struct net_bridge *br, str
>
> kobject_uevent(&p->kobj, KOBJ_ADD);
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (br_devices_support_netpoll(br)) {
> + br->dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
> + if (br->dev->npinfo)
> + dev->npinfo = br->dev->npinfo;
> + } else if (!(br->dev->priv_flags & IFF_DISABLE_NETPOLL)) {
> + br->dev->priv_flags |= IFF_DISABLE_NETPOLL;
> + printk(KERN_INFO "New device %s does not support netpoll\n",
> + dev->name);
> + printk(KERN_INFO "Disabling netpoll for %s\n",
> + br->dev->name);
One message is sufficient.
--
--
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