Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750858Ab0D1ECw (ORCPT ); Wed, 28 Apr 2010 00:02:52 -0400 Received: from mail-pz0-f204.google.com ([209.85.222.204]:37147 "EHLO mail-pz0-f204.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750744Ab0D1ECu (ORCPT ); Wed, 28 Apr 2010 00:02:50 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=xAweW2DEl3E8jdEkK+n8+4oMmmtTEh134+rEqN3sYu3iJsXjhZku4aOoQPko72qdzn o/t9prPassntgDyPu+VPklkwkxp/4UabW9igqYj1UvHLsKN3gkSIibjaP0ukN8JJMTMs RvlRbxiFqlq/gOuSe8F8BMCkcEfOGkWG+hBIE= MIME-Version: 1.0 In-Reply-To: <20100427075937.4908.18468.sendpatchset@localhost.localdomain> References: <20100427075937.4908.18468.sendpatchset@localhost.localdomain> Date: Wed, 28 Apr 2010 12:02:49 +0800 Message-ID: Subject: Re: [v4 Patch 1/3] netpoll: add generic support for bridge and bonding devices From: Dongdong Deng To: Amerigo Wang Cc: linux-kernel@vger.kernel.org, Matt Mackall , netdev@vger.kernel.org, bridge@lists.linux-foundation.org, Andy Gospodarek , Neil Horman , Jeff Moyer , Stephen Hemminger , bonding-devel@lists.sourceforge.net, Jay Vosburgh , David Miller Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by alpha.home.local id o3S430mE018548 Content-Length: 12016 Lines: 7 On Tue, Apr 27, 2010 at 3:55 PM, Amerigo Wang 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 > Cc: Neil Horman > Signed-off-by: WANG Cong >> --->> 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 havethe netpoll_cleanup opt. RegardsDongdong >                        }>                }>> @@ -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@vger.kernel.org> More majordomo info at  http://vger.kernel.org/majordomo-info.html> Please read the FAQ at  http://www.tux.org/lkml/>????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?