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

From: Dongdong Deng
Date: Wed Apr 28 2010 - 00:02:55 EST

Next message: Changli Gao: "[PATCH] sched: use wrapper functions"
Previous message: Xiao Guangrong: "[PATCH v3 10/10] KVM MMU: optimize sync/update unsync-page"
In reply to: David Miller: "Re: [v4 Patch 1/3] netpoll: add generic support for bridge andbonding devices"
Next in thread: Cong Wang: "Re: [v4 Patch 1/3] netpoll: add generic support for bridge and bondingdevices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Apr 27, 2010 at 3:55 PM, Amerigo Wang <amwang@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
> Cc: Neil Horman <nhorman@xxxxxxxxxxxxx>
> Signed-off-by: WANG Cong <amwang@xxxxxxxxxx>
>
> ---
>
> 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@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
> Please read the FAQ at Âhttp://www.tux.org/lkml/
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~††Ûiÿûàz¹®w¥¢¸?™¨èÚ&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i

Next message: Changli Gao: "[PATCH] sched: use wrapper functions"
Previous message: Xiao Guangrong: "[PATCH v3 10/10] KVM MMU: optimize sync/update unsync-page"
In reply to: David Miller: "Re: [v4 Patch 1/3] netpoll: add generic support for bridge andbonding devices"
Next in thread: Cong Wang: "Re: [v4 Patch 1/3] netpoll: add generic support for bridge and bondingdevices"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]