Stephen Hemminger
2018-May-31 02:06 UTC
[PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
On Thu, 24 May 2018 09:55:14 -0700 Sridhar Samudrala <sridhar.samudrala at intel.com> wrote:> Use the registration/notification framework supported by the generic > failover infrastructure. > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com>Why was this merged? It was never signed off by any of the netvsc maintainers, and there were still issues unresolved. There are also namespaces issues I am fixing and this breaks them. Will start my patch set with a revert for this. Sorry
Samudrala, Sridhar
2018-May-31 03:03 UTC
[PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
On 5/30/2018 7:06 PM, Stephen Hemminger wrote:> On Thu, 24 May 2018 09:55:14 -0700 > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > >> Use the registration/notification framework supported by the generic >> failover infrastructure. >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > Why was this merged? It was never signed off by any of the netvsc maintainers, > and there were still issues unresolved. > > There are also namespaces issues I am fixing and this breaks them. > Will start my patch set with a revert for this. SorryI would appreciate if you can make the fixes on top of this patch series. I tried hard to make sure that netvsc functionality and behavior doesn't change. It is possible that there could be some bugs introduced, but they can be fixed. Looks like Wei already found a bug and submitted a fix for that.
Stephen Hemminger
2018-May-31 12:58 UTC
[PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
On Wed, 30 May 2018 20:03:11 -0700 "Samudrala, Sridhar" <sridhar.samudrala at intel.com> wrote:> On 5/30/2018 7:06 PM, Stephen Hemminger wrote: > > On Thu, 24 May 2018 09:55:14 -0700 > > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > > > >> Use the registration/notification framework supported by the generic > >> failover infrastructure. > >> > >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > > Why was this merged? It was never signed off by any of the netvsc maintainers, > > and there were still issues unresolved. > > > > There are also namespaces issues I am fixing and this breaks them. > > Will start my patch set with a revert for this. Sorry > > I would appreciate if you can make the fixes on top of this patch series. I tried hard > to make sure that netvsc functionality and behavior doesn't change. > > It is possible that there could be some bugs introduced, but they can be fixed. > Looks like Wei already found a bug and submitted a fix for that. >Ok, but several of these may clash with what you want for virtio. Like: - VF should be moved to namespace of virt device - VF should be associated based on message from host with serial # not registration notifier and MAC address. - control operations should use master device reference rather than searching based on MAC. As you can see these are structural changes.
Michael S. Tsirkin
2018-May-31 18:35 UTC
[PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
On Wed, May 30, 2018 at 10:06:35PM -0400, Stephen Hemminger wrote:> On Thu, 24 May 2018 09:55:14 -0700 > Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > > > Use the registration/notification framework supported by the generic > > failover infrastructure. > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> > > Why was this merged? It was never signed off by any of the netvsc maintainers, > and there were still issues unresolved. > > There are also namespaces issues I am fixing and this breaks them. > Will start my patch set with a revert for this. SorryAs long as you finish the patch set with re-integrating with failover, that's fine IMHO. I suspect it's easier to add the code to failover though - namespace things likely affect virtio as well. Lookup by ID would be an optional feature for virtio, but probably a useful one - I won't ask you to add it to virtio but it could be a mode in failover that virtio will activate down the road. And reducing the number of times we look cards up based on ID can only be a good thing. -- MST
Siwei Liu
2018-May-31 20:41 UTC
[PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
On Thu, May 31, 2018 at 11:35 AM, Michael S. Tsirkin <mst at redhat.com> wrote:> On Wed, May 30, 2018 at 10:06:35PM -0400, Stephen Hemminger wrote: >> On Thu, 24 May 2018 09:55:14 -0700 >> Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: >> >> > Use the registration/notification framework supported by the generic >> > failover infrastructure. >> > >> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala at intel.com> >> >> Why was this merged? It was never signed off by any of the netvsc maintainers, >> and there were still issues unresolved. >> >> There are also namespaces issues I am fixing and this breaks them. >> Will start my patch set with a revert for this. Sorry > > As long as you finish the patch set with re-integrating with failover, > that's fine IMHO. > > I suspect it's easier to add the code to failover though - namespace > things likely affect virtio as well. Lookup by ID would be an optional > feature for virtio, but probably a useful one - I won't ask youI would think for production uses this is a required feature and should be enabled by default. Venu (cc'ed) is working on the group ID stuff currently for pairing virtio and passthrough devices. Would appreciate feedback on the group ID proposal on virtio-dev. -Siwei> to add it to virtio but it could be a mode in failover > that virtio will activate down the road. And reducing the number of > times we look cards up based on ID can only be a good thing. > > -- > MST