| Message ID | 1404811947-5174-1-git-send-email-richard@nod.at |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
> -----Original Message----- > From: Richard Weinberger [mailto:richard@nod.at] > Sent: Tuesday, July 8, 2014 5:32 AM > To: KY Srinivasan; Haiyang Zhang > Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; Richard Weinberger > Subject: [PATCH] hyperv: Add netpoll support > > In order to have at least a netconsole to debug kernel issues on > Windows Azure this patch implements netpoll support. > Sending packets is easy, netvsc_start_xmit() does already everything > needed. > To receive we need to trigger the channel callback which is usally > called via tasklet_schedule(). > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c > index 4fd71b7..367b71e 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device > *ndev, void *p) > return err; > } > > +#ifdef CONFIG_NET_POLL_CONTROLLER > +static void netvsc_poll_controller(struct net_device *net) > +{ > + struct net_device_context *net_device_ctx = netdev_priv(net); > + struct hv_device *dev = net_device_ctx->device_ctx; > + > + local_bh_disable(); > + netvsc_channel_cb(dev->channel); This can only poll the primary channel not the sub channels. Thanks, - Haiyang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Richard Weinberger [mailto:richard@nod.at] > Sent: Tuesday, July 8, 2014 2:32 AM > To: KY Srinivasan; Haiyang Zhang > Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; Richard Weinberger > Subject: [PATCH] hyperv: Add netpoll support > > In order to have at least a netconsole to debug kernel issues on Windows > Azure this patch implements netpoll support. > Sending packets is easy, netvsc_start_xmit() does already everything > needed. > To receive we need to trigger the channel callback which is usally called via > tasklet_schedule(). > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/net/hyperv/netvsc_drv.c > b/drivers/net/hyperv/netvsc_drv.c index 4fd71b7..367b71e 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device > *ndev, void *p) > return err; > } > > +#ifdef CONFIG_NET_POLL_CONTROLLER > +static void netvsc_poll_controller(struct net_device *net) { > + struct net_device_context *net_device_ctx = netdev_priv(net); > + struct hv_device *dev = net_device_ctx->device_ctx; > + > + local_bh_disable(); > + netvsc_channel_cb(dev->channel); > + local_bh_enable(); > +} > +#endif Each channel is bound to a specific VCPU in the guest and the channel callback is expected to be delivered on the VCPU the channel is bound to. This code is not satisfying that requirement. Regards, K. Y -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 08.07.2014 20:01, schrieb KY Srinivasan: > > >> -----Original Message----- >> From: Richard Weinberger [mailto:richard@nod.at] >> Sent: Tuesday, July 8, 2014 2:32 AM >> To: KY Srinivasan; Haiyang Zhang >> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- >> kernel@vger.kernel.org; Richard Weinberger >> Subject: [PATCH] hyperv: Add netpoll support >> >> In order to have at least a netconsole to debug kernel issues on Windows >> Azure this patch implements netpoll support. >> Sending packets is easy, netvsc_start_xmit() does already everything >> needed. >> To receive we need to trigger the channel callback which is usally called via >> tasklet_schedule(). >> >> Signed-off-by: Richard Weinberger <richard@nod.at> >> --- >> drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/net/hyperv/netvsc_drv.c >> b/drivers/net/hyperv/netvsc_drv.c index 4fd71b7..367b71e 100644 >> --- a/drivers/net/hyperv/netvsc_drv.c >> +++ b/drivers/net/hyperv/netvsc_drv.c >> @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device >> *ndev, void *p) >> return err; >> } >> >> +#ifdef CONFIG_NET_POLL_CONTROLLER >> +static void netvsc_poll_controller(struct net_device *net) { >> + struct net_device_context *net_device_ctx = netdev_priv(net); >> + struct hv_device *dev = net_device_ctx->device_ctx; >> + >> + local_bh_disable(); >> + netvsc_channel_cb(dev->channel); >> + local_bh_enable(); >> +} >> +#endif > > Each channel is bound to a specific VCPU in the guest and the channel callback is expected to be delivered on > the VCPU the channel is bound to. This code is not satisfying that requirement. But struct hv_device has only one channel attribute. How does this work with multiple VCPUs? Anyways, what solution to you propose? Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 08.07.2014 19:55, schrieb Haiyang Zhang: > > >> -----Original Message----- >> From: Richard Weinberger [mailto:richard@nod.at] >> Sent: Tuesday, July 8, 2014 5:32 AM >> To: KY Srinivasan; Haiyang Zhang >> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- >> kernel@vger.kernel.org; Richard Weinberger >> Subject: [PATCH] hyperv: Add netpoll support >> >> In order to have at least a netconsole to debug kernel issues on >> Windows Azure this patch implements netpoll support. >> Sending packets is easy, netvsc_start_xmit() does already everything >> needed. >> To receive we need to trigger the channel callback which is usally >> called via tasklet_schedule(). >> >> Signed-off-by: Richard Weinberger <richard@nod.at> >> --- >> drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/net/hyperv/netvsc_drv.c >> b/drivers/net/hyperv/netvsc_drv.c >> index 4fd71b7..367b71e 100644 >> --- a/drivers/net/hyperv/netvsc_drv.c >> +++ b/drivers/net/hyperv/netvsc_drv.c >> @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device >> *ndev, void *p) >> return err; >> } >> >> +#ifdef CONFIG_NET_POLL_CONTROLLER >> +static void netvsc_poll_controller(struct net_device *net) >> +{ >> + struct net_device_context *net_device_ctx = netdev_priv(net); >> + struct hv_device *dev = net_device_ctx->device_ctx; >> + >> + local_bh_disable(); >> + netvsc_channel_cb(dev->channel); > > This can only poll the primary channel not the sub channels. Sub channels in terms of one channel per VCPU as KY said? *confused*, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Richard Weinberger [mailto:richard@nod.at] > Sent: Tuesday, July 8, 2014 2:40 PM > To: Haiyang Zhang; KY Srinivasan > Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] hyperv: Add netpoll support > > Am 08.07.2014 19:55, schrieb Haiyang Zhang: > > > > > >> -----Original Message----- > >> From: Richard Weinberger [mailto:richard@nod.at] > >> Sent: Tuesday, July 8, 2014 5:32 AM > >> To: KY Srinivasan; Haiyang Zhang > >> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > >> kernel@vger.kernel.org; Richard Weinberger > >> Subject: [PATCH] hyperv: Add netpoll support > >> > >> In order to have at least a netconsole to debug kernel issues on > >> Windows Azure this patch implements netpoll support. > >> Sending packets is easy, netvsc_start_xmit() does already everything > >> needed. > >> To receive we need to trigger the channel callback which is usally > >> called via tasklet_schedule(). > >> > >> Signed-off-by: Richard Weinberger <richard@nod.at> > >> --- > >> drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/drivers/net/hyperv/netvsc_drv.c > >> b/drivers/net/hyperv/netvsc_drv.c > >> index 4fd71b7..367b71e 100644 > >> --- a/drivers/net/hyperv/netvsc_drv.c > >> +++ b/drivers/net/hyperv/netvsc_drv.c > >> @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device > >> *ndev, void *p) > >> return err; > >> } > >> > >> +#ifdef CONFIG_NET_POLL_CONTROLLER > >> +static void netvsc_poll_controller(struct net_device *net) > >> +{ > >> + struct net_device_context *net_device_ctx = netdev_priv(net); > >> + struct hv_device *dev = net_device_ctx->device_ctx; > >> + > >> + local_bh_disable(); > >> + netvsc_channel_cb(dev->channel); > > > > This can only poll the primary channel not the sub channels. > > Sub channels in terms of one channel per VCPU as KY said? > > *confused*, Since it's used only for debugging, polling the subchannels may not be necessary. Regarding the CPU binding, KY will reply you in another email. Thanks, - Haiyang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Richard Weinberger [mailto:richard@nod.at] > Sent: Tuesday, July 8, 2014 11:39 AM > To: KY Srinivasan; Haiyang Zhang > Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] hyperv: Add netpoll support > > Am 08.07.2014 20:01, schrieb KY Srinivasan: > > > > > >> -----Original Message----- > >> From: Richard Weinberger [mailto:richard@nod.at] > >> Sent: Tuesday, July 8, 2014 2:32 AM > >> To: KY Srinivasan; Haiyang Zhang > >> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org; linux- > >> kernel@vger.kernel.org; Richard Weinberger > >> Subject: [PATCH] hyperv: Add netpoll support > >> > >> In order to have at least a netconsole to debug kernel issues on > >> Windows Azure this patch implements netpoll support. > >> Sending packets is easy, netvsc_start_xmit() does already everything > >> needed. > >> To receive we need to trigger the channel callback which is usally > >> called via tasklet_schedule(). > >> > >> Signed-off-by: Richard Weinberger <richard@nod.at> > >> --- > >> drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/drivers/net/hyperv/netvsc_drv.c > >> b/drivers/net/hyperv/netvsc_drv.c index 4fd71b7..367b71e 100644 > >> --- a/drivers/net/hyperv/netvsc_drv.c > >> +++ b/drivers/net/hyperv/netvsc_drv.c > >> @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device > >> *ndev, void *p) > >> return err; > >> } > >> > >> +#ifdef CONFIG_NET_POLL_CONTROLLER > >> +static void netvsc_poll_controller(struct net_device *net) { > >> + struct net_device_context *net_device_ctx = netdev_priv(net); > >> + struct hv_device *dev = net_device_ctx->device_ctx; > >> + > >> + local_bh_disable(); > >> + netvsc_channel_cb(dev->channel); > >> + local_bh_enable(); > >> +} > >> +#endif > > > > Each channel is bound to a specific VCPU in the guest and the channel > > callback is expected to be delivered on the VCPU the channel is bound to. > This code is not satisfying that requirement. > > But struct hv_device has only one channel attribute. How does this work with > multiple VCPUs? > > Anyways, what solution to you propose? The VCPU the channel is bound to is available in the channel state. You could use the following code Fragment to ensure that the call is made on the "right" cpu: smp_call_function_single(dev->channel->target_cpu, netvsc_channel_cb, dev->channel, true); Hope this helps. K. Y -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 08.07.2014 22:03, schrieb KY Srinivasan: > The VCPU the channel is bound to is available in the channel state. You could use the following code > Fragment to ensure that the call is made on the "right" cpu: > > smp_call_function_single(dev->channel->target_cpu, > netvsc_channel_cb, dev->channel, true); This won't work as netpoll runs with IRQs disabled. ->ndo_poll_controller() has to make sure that SKBs can be received and transmitted while IRQs are off. I thought calling the channel callback by hand would be enough to receive SKBs. Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Weinberger <richard@nod.at> : [...] > This won't work as netpoll runs with IRQs disabled. > ->ndo_poll_controller() has to make sure that SKBs can be received and transmitted > while IRQs are off. I thought calling the channel callback by hand would be > enough to receive SKBs. What are you taking about ? netconsole does not need to receive. hyperv start_xmit handler almost does its own Tx completion as you have noticed. The situation is imho close to a virtual device one as was veth in bb446c19fefd7b4435adb12a9dd7666adc5b553a.
Am 09.07.2014 00:47, schrieb Francois Romieu: > Richard Weinberger <richard@nod.at> : > [...] >> This won't work as netpoll runs with IRQs disabled. >> ->ndo_poll_controller() has to make sure that SKBs can be received and transmitted >> while IRQs are off. I thought calling the channel callback by hand would be >> enough to receive SKBs. > > What are you taking about ? netconsole does not need to receive. Isn't netconsole is only one user of netpoll? Of course netconsole needs only to transmit SKBs. But if you look at other ->ndo_poll_controller implementations you'll notice that they care also about receiving. > hyperv start_xmit handler almost does its own Tx completion as you have > noticed. The situation is imho close to a virtual device one as was veth > in bb446c19fefd7b4435adb12a9dd7666adc5b553a. Bad commit reference: bb446c19fefd7b4435adb12a9dd7666adc5b553a :-( Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 09.07.2014 00:47, schrieb Francois Romieu: > Richard Weinberger <richard@nod.at> : > [...] >> This won't work as netpoll runs with IRQs disabled. >> ->ndo_poll_controller() has to make sure that SKBs can be received and transmitted >> while IRQs are off. I thought calling the channel callback by hand would be >> enough to receive SKBs. > > What are you taking about ? netconsole does not need to receive. > > hyperv start_xmit handler almost does its own Tx completion as you have > noticed. The situation is imho close to a virtual device one as was veth > in bb446c19fefd7b4435adb12a9dd7666adc5b553a. Ah, net-next.git. My first (in-house) patch had the same empty poll controller as tun.c and now veth.c have. If we are fine with tx only, I'll happily resend an updated patch with an empty poll controller. :-) Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Weinberger <richard@nod.at> : > Am 09.07.2014 00:47, schrieb Francois Romieu: [...] > > What are you taking about ? netconsole does not need to receive. > > Isn't netconsole is only one user of netpoll ? Out of tree users are irrelevant. See netpoll related comments in cd6362befe4cc7bf589a5236d2a780af2d47bcc9 > Of course netconsole needs only to transmit SKBs. > But if you look at other ->ndo_poll_controller implementations > you'll notice that they care also about receiving. It's just the long, illuminating history of netpoll :o) Some limited Rx netpoll support may be done but it needs more work than was originally advertised. > > hyperv start_xmit handler almost does its own Tx completion as you have > > noticed. The situation is imho close to a virtual device one as was veth > > in bb446c19fefd7b4435adb12a9dd7666adc5b553a. > > Bad commit reference: bb446c19fefd7b4435adb12a9dd7666adc5b553a Sorry, it currently belongs to davem's net-next.
Am 09.07.2014 01:43, schrieb Francois Romieu: > Richard Weinberger <richard@nod.at> : >> Am 09.07.2014 00:47, schrieb Francois Romieu: > [...] >>> What are you taking about ? netconsole does not need to receive. >> >> Isn't netconsole is only one user of netpoll ? > > Out of tree users are irrelevant. See netpoll related comments in > cd6362befe4cc7bf589a5236d2a780af2d47bcc9 Thanks lot for pointing this out! >> Of course netconsole needs only to transmit SKBs. >> But if you look at other ->ndo_poll_controller implementations >> you'll notice that they care also about receiving. > > It's just the long, illuminating history of netpoll :o) > > Some limited Rx netpoll support may be done but it needs more work > than was originally advertised. > >>> hyperv start_xmit handler almost does its own Tx completion as you have >>> noticed. The situation is imho close to a virtual device one as was veth >>> in bb446c19fefd7b4435adb12a9dd7666adc5b553a. >> >> Bad commit reference: bb446c19fefd7b4435adb12a9dd7666adc5b553a > > Sorry, it currently belongs to davem's net-next. Found it already. :) Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 4fd71b7..367b71e 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -736,6 +736,17 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p) return err; } +#ifdef CONFIG_NET_POLL_CONTROLLER +static void netvsc_poll_controller(struct net_device *net) +{ + struct net_device_context *net_device_ctx = netdev_priv(net); + struct hv_device *dev = net_device_ctx->device_ctx; + + local_bh_disable(); + netvsc_channel_cb(dev->channel); + local_bh_enable(); +} +#endif static const struct ethtool_ops ethtool_ops = { .get_drvinfo = netvsc_get_drvinfo, @@ -751,6 +762,9 @@ static const struct net_device_ops device_ops = { .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = netvsc_set_mac_addr, .ndo_select_queue = netvsc_select_queue, +#ifdef CONFIG_NET_POLL_CONTROLLER + .ndo_poll_controller = netvsc_poll_controller, +#endif }; /*
In order to have at least a netconsole to debug kernel issues on Windows Azure this patch implements netpoll support. Sending packets is easy, netvsc_start_xmit() does already everything needed. To receive we need to trigger the channel callback which is usally called via tasklet_schedule(). Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/net/hyperv/netvsc_drv.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)