[Bridge] [v4 Patch 1/3] netpoll: add generic support for bridge and bonding devices

Dongdong Deng libfetion at gmail.com
Tue Apr 27 21:02:49 PDT 2010


On Tue, Apr 27, 2010 at 3:55 PM, Amerigo Wang <amwang at redhat.com> wrote:
> V4:
> Use "unlikely" to mark netpoll call path, suggested by Stephen.
> Handle NETDEV_GOING_DOWN case.
>
> V3:
> Update to latest Linus' tree.
> Fix deadlocks when releasing slaves of bonding devices.
> Thanks to Andy.
>
> V2:
> Fix some bugs of previous version.
> Remove ->netpoll_setup and ->netpoll_xmit, they are not necessary.
> Don't poll all underlying devices, poll ->real_dev in struct netpoll.
> Thanks to David for suggesting above.
>
> ------------>
>
> This whole patchset is for adding netpoll support to bridge and bonding
> devices. I already tested it for bridge, bonding, bridge over bonding,
> and bonding over bridge. It looks fine now.
>
>
> To make bridge and bonding support netpoll, we need to adjust
> some netpoll generic code. This patch does the following things:
>
> 1) introduce two new priv_flags for struct net_device:
>   IFF_IN_NETPOLL which identifies we are processing a netpoll;
>   IFF_DISABLE_NETPOLL is used to disable netpoll support for a device
>   at run-time;
>
> 2) introduce one new method for netdev_ops:
>   ->ndo_netpoll_cleanup() is used to clean up netpoll when a device is
>     removed.
>
> 3) introduce netpoll_poll_dev() which takes a struct net_device * parameter;
>   export netpoll_send_skb() and netpoll_poll_dev() which will be used later;
>
> 4) hide a pointer to struct netpoll in struct netpoll_info, ditto.
>
> 5) introduce ->real_dev for struct netpoll.
>
> 6) introduce a new status NETDEV_BONDING_DESLAE, which is used to disable
>   netconsole before releasing a slave, to avoid deadlocks.
>
> Cc: David Miller <davem at davemloft.net>
> Cc: Neil Horman <nhorman at tuxdriver.com>
> Signed-off-by: WANG Cong <amwang at redhat.com>
>
> ---
>
> Index: linux-2.6/include/linux/if.h
> ===================================================================
> --- linux-2.6.orig/include/linux/if.h
> +++ linux-2.6/include/linux/if.h
> @@ -71,6 +71,8 @@
>                                         * release skb->dst
>                                         */
>  #define IFF_DONT_BRIDGE 0x800          /* disallow bridging this ether dev */
> +#define IFF_IN_NETPOLL 0x1000          /* whether we are processing netpoll */
> +#define IFF_DISABLE_NETPOLL    0x2000  /* disable netpoll at run-time */
>
>  #define IF_GET_IFACE   0x0001          /* for querying only */
>  #define IF_GET_PROTO   0x0002
> Index: linux-2.6/include/linux/netdevice.h
> ===================================================================
> --- linux-2.6.orig/include/linux/netdevice.h
> +++ linux-2.6/include/linux/netdevice.h
> @@ -667,6 +667,7 @@ struct net_device_ops {
>                                                        unsigned short vid);
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>        void                    (*ndo_poll_controller)(struct net_device *dev);
> +       void                    (*ndo_netpoll_cleanup)(struct net_device *dev);
>  #endif
>        int                     (*ndo_set_vf_mac)(struct net_device *dev,
>                                                  int queue, u8 *mac);
> Index: linux-2.6/include/linux/netpoll.h
> ===================================================================
> --- linux-2.6.orig/include/linux/netpoll.h
> +++ linux-2.6/include/linux/netpoll.h
> @@ -14,6 +14,7 @@
>
>  struct netpoll {
>        struct net_device *dev;
> +       struct net_device *real_dev;
>        char dev_name[IFNAMSIZ];
>        const char *name;
>        void (*rx_hook)(struct netpoll *, int, char *, int);
> @@ -36,8 +37,11 @@ struct netpoll_info {
>        struct sk_buff_head txq;
>
>        struct delayed_work tx_work;
> +
> +       struct netpoll *netpoll;
>  };
>
> +void netpoll_poll_dev(struct net_device *dev);
>  void netpoll_poll(struct netpoll *np);
>  void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
>  void netpoll_print_options(struct netpoll *np);
> @@ -47,6 +51,7 @@ int netpoll_trap(void);
>  void netpoll_set_trap(int trap);
>  void netpoll_cleanup(struct netpoll *np);
>  int __netpoll_rx(struct sk_buff *skb);
> +void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
>
>
>  #ifdef CONFIG_NETPOLL
> Index: linux-2.6/net/core/netpoll.c
> ===================================================================
> --- linux-2.6.orig/net/core/netpoll.c
> +++ linux-2.6/net/core/netpoll.c
> @@ -179,9 +179,8 @@ static void service_arp_queue(struct net
>        }
>  }
>
> -void netpoll_poll(struct netpoll *np)
> +void netpoll_poll_dev(struct net_device *dev)
>  {
> -       struct net_device *dev = np->dev;
>        const struct net_device_ops *ops;
>
>        if (!dev || !netif_running(dev))
> @@ -201,6 +200,11 @@ void netpoll_poll(struct netpoll *np)
>        zap_completion_queue();
>  }
>
> +void netpoll_poll(struct netpoll *np)
> +{
> +       netpoll_poll_dev(np->dev);
> +}
> +
>  static void refill_skbs(void)
>  {
>        struct sk_buff *skb;
> @@ -282,7 +286,7 @@ static int netpoll_owner_active(struct n
>        return 0;
>  }
>
> -static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
> +void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
>  {
>        int status = NETDEV_TX_BUSY;
>        unsigned long tries;
> @@ -308,7 +312,9 @@ static void netpoll_send_skb(struct netp
>                     tries > 0; --tries) {
>                        if (__netif_tx_trylock(txq)) {
>                                if (!netif_tx_queue_stopped(txq)) {
> +                                       dev->priv_flags |= IFF_IN_NETPOLL;
>                                        status = ops->ndo_start_xmit(skb, dev);
> +                                       dev->priv_flags &= ~IFF_IN_NETPOLL;
>                                        if (status == NETDEV_TX_OK)
>                                                txq_trans_update(txq);
>                                }
> @@ -756,7 +762,10 @@ int netpoll_setup(struct netpoll *np)
>                atomic_inc(&npinfo->refcnt);
>        }
>
> -       if (!ndev->netdev_ops->ndo_poll_controller) {
> +       npinfo->netpoll = np;
> +
> +       if (ndev->priv_flags & IFF_DISABLE_NETPOLL
> +                       || !ndev->netdev_ops->ndo_poll_controller) {
>                printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
>                       np->name, np->dev_name);
>                err = -ENOTSUPP;
> @@ -878,6 +887,7 @@ void netpoll_cleanup(struct netpoll *np)
>                        }
>
>                        if (atomic_dec_and_test(&npinfo->refcnt)) {
> +                               const struct net_device_ops *ops;
>                                skb_queue_purge(&npinfo->arp_tx);
>                                skb_queue_purge(&npinfo->txq);
>                                cancel_rearming_delayed_work(&npinfo->tx_work);
> @@ -885,7 +895,11 @@ void netpoll_cleanup(struct netpoll *np)
>                                /* clean after last, unfinished work */
>                                __skb_queue_purge(&npinfo->txq);
>                                kfree(npinfo);
> -                               np->dev->npinfo = NULL;
> +                               ops = np->dev->netdev_ops;
> +                               if (ops->ndo_netpoll_cleanup)
> +                                       ops->ndo_netpoll_cleanup(np->dev);
> +                               else
> +                                       np->dev->npinfo = NULL;


+             if (ops->ndo_netpoll_cleanup)
+                                       ops->ndo_netpoll_cleanup(np->dev);
+             np->dev->npinfo = NULL;

I think it is good to set np->dev->npinfo to NULL  even though we have
the netpoll_cleanup opt.

Regards
Dongdong

>                        }
>                }
>
> @@ -908,6 +922,7 @@ void netpoll_set_trap(int trap)
>                atomic_dec(&trapped);
>  }
>
> +EXPORT_SYMBOL(netpoll_send_skb);
>  EXPORT_SYMBOL(netpoll_set_trap);
>  EXPORT_SYMBOL(netpoll_trap);
>  EXPORT_SYMBOL(netpoll_print_options);
> @@ -915,4 +930,5 @@ EXPORT_SYMBOL(netpoll_parse_options);
>  EXPORT_SYMBOL(netpoll_setup);
>  EXPORT_SYMBOL(netpoll_cleanup);
>  EXPORT_SYMBOL(netpoll_send_udp);
> +EXPORT_SYMBOL(netpoll_poll_dev);
>  EXPORT_SYMBOL(netpoll_poll);
> Index: linux-2.6/drivers/net/netconsole.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/netconsole.c
> +++ linux-2.6/drivers/net/netconsole.c
> @@ -665,7 +665,8 @@ static int netconsole_netdev_event(struc
>        struct netconsole_target *nt;
>        struct net_device *dev = ptr;
>
> -       if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER))
> +       if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
> +             event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
>                goto done;
>
>        spin_lock_irqsave(&target_list_lock, flags);
> @@ -677,19 +678,21 @@ static int netconsole_netdev_event(struc
>                                strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
>                                break;
>                        case NETDEV_UNREGISTER:
> -                               if (!nt->enabled)
> -                                       break;
>                                netpoll_cleanup(&nt->np);
> +                               /* Fall through */
> +                       case NETDEV_GOING_DOWN:
> +                       case NETDEV_BONDING_DESLAVE:
>                                nt->enabled = 0;
> -                               printk(KERN_INFO "netconsole: network logging stopped"
> -                                       ", interface %s unregistered\n",
> -                                       dev->name);
>                                break;
>                        }
>                }
>                netconsole_target_put(nt);
>        }
>        spin_unlock_irqrestore(&target_list_lock, flags);
> +       if (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE)
> +               printk(KERN_INFO "netconsole: network logging stopped, "
> +                       "interface %s %s\n",  dev->name,
> +                       event == NETDEV_UNREGISTER ? "unregistered" : "released slaves");
>
>  done:
>        return NOTIFY_DONE;
> Index: linux-2.6/include/linux/notifier.h
> ===================================================================
> --- linux-2.6.orig/include/linux/notifier.h
> +++ linux-2.6/include/linux/notifier.h
> @@ -203,6 +203,7 @@ static inline int notifier_to_errno(int
>  #define NETDEV_BONDING_NEWTYPE  0x000F
>  #define NETDEV_POST_INIT       0x0010
>  #define NETDEV_UNREGISTER_BATCH 0x0011
> +#define NETDEV_BONDING_DESLAVE  0x0012
>
>  #define SYS_DOWN       0x0001  /* Notify of system down */
>  #define SYS_RESTART    SYS_DOWN
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


More information about the Bridge mailing list