search for: iff_master

On Thu, 24 May 2018 09:55:13 -0700 Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 03ed492c4e14..0f4ba52b641d 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1421,6 +1421,8 @@ struct net_device_ops { > * entity (i.e. the master device for bridged veth)

On Thu, 24 May 2018 09:55:13 -0700 Sridhar Samudrala <sridhar.samudrala at intel.com> wrote: > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 03ed492c4e14..0f4ba52b641d 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1421,6 +1421,8 @@ struct net_device_ops { > * entity (i.e. the master device for bridged veth)

Sat, Apr 28, 2018 at 11:06:01AM CEST, jiri at resnulli.us wrote: >Fri, Apr 27, 2018 at 07:06:58PM CEST, sridhar.samudrala at intel.com wrote: [...] >>+ >>+ err = netdev_rx_handler_register(slave_dev, net_failover_handle_frame, >>+ failover_dev); >>+ if (err) { >>+ netdev_err(slave_dev, "can not register failover rx handler (err = %d)\n",

Sat, Apr 28, 2018 at 11:06:01AM CEST, jiri at resnulli.us wrote: >Fri, Apr 27, 2018 at 07:06:58PM CEST, sridhar.samudrala at intel.com wrote: [...] >>+ >>+ err = netdev_rx_handler_register(slave_dev, net_failover_handle_frame, >>+ failover_dev); >>+ if (err) { >>+ netdev_err(slave_dev, "can not register failover rx handler (err = %d)\n",

...7, > >> + IFF_FAILOVER_SLAVE = 1<<28, > >> }; > > Why is FAILOVER any different than other master/slave relationships. > > I don't think you need to take up precious netdev flag bits for this. > > These are netdev priv flags. > Jiri says that IFF_MASTER/IFF_SLAVE are bonding specific flags and cannot be used > with other failover mechanisms. Team also doesn't use this flags and it has its own > priv_flags. > This change breaks userspace. We already have worked with partners to ignore devices marked as IFF_SLAVE, and IFF_SLAVE is vis...

...ill look into it.? Do you think this will help with the issue > you saw with having to change mac on standy twice to get the init scripts > working? We are now going to block changing the mac on both standby and > failover. > > Also, i was wondering if we should set dev->flags to IFF_MASTER on failover > and IFF_SLAVE on primary and standby. We do need a way to find things out, that's for sure. How does userspace know it's a failover config and find the failover device right now? > netvsc does this. > Does this help with the init scripts and network manager to skip...

...+ /* Avoid Vlan dev with same MAC registering as VF */ > + if (is_vlan_dev(event_dev)) > + return NOTIFY_DONE; > + > + /* Avoid Bonding master dev with same MAC registering as VF */ > + if ((event_dev->priv_flags & IFF_BONDING) && > + (event_dev->flags & IFF_MASTER)) > + return NOTIFY_DONE; > + > + switch (event) { > + case NETDEV_REGISTER: > + return virtnet_register_vf(event_dev); > + case NETDEV_UNREGISTER: > + return virtnet_unregister_vf(event_dev); > + default: > + return NOTIFY_DONE; > + } > +} > + > +static s...

...> + SET_NETDEV_DEV(bypass_netdev, dev); >> + >> + bypass_netdev->netdev_ops = &virtnet_bypass_netdev_ops; >> + bypass_netdev->ethtool_ops = &virtnet_bypass_ethtool_ops; >> + >> + /* Initialize the device options */ >> + bypass_netdev->flags |= IFF_MASTER; > I think I pointed that out already. Don't use "IFF_MASTER". That is > specific to bonding. As I suggested in the reply to the patch #2, you > should introduce IFF_BYPASS. Also, this flag should be set by the bypass > module. Just create the netdev and do things specific...

...lave changes > Sure. will look into it.? Do you think this will help with the issue you saw with having to change mac on standy twice to get the init scripts working? We are now going to block changing the mac on both standby and failover. Also, i was wondering if we should set dev->flags to IFF_MASTER on failover and IFF_SLAVE on primary and standby. netvsc does this. Does this help with the init scripts and network manager to skip slave devices for dhcp requests?

...>> + IFF_FAILOVER = 1<<27, >> + IFF_FAILOVER_SLAVE = 1<<28, >> }; > Why is FAILOVER any different than other master/slave relationships. > I don't think you need to take up precious netdev flag bits for this. These are netdev priv flags. Jiri says that IFF_MASTER/IFF_SLAVE are bonding specific flags and cannot be used with other failover mechanisms. Team also doesn't use this flags and it has its own priv_flags.

...tdev, dev_net(backup_netdev)); >+ SET_NETDEV_DEV(bypass_netdev, dev); >+ >+ bypass_netdev->netdev_ops = &virtnet_bypass_netdev_ops; >+ bypass_netdev->ethtool_ops = &virtnet_bypass_ethtool_ops; >+ >+ /* Initialize the device options */ >+ bypass_netdev->flags |= IFF_MASTER; I think I pointed that out already. Don't use "IFF_MASTER". That is specific to bonding. As I suggested in the reply to the patch #2, you should introduce IFF_BYPASS. Also, this flag should be set by the bypass module. Just create the netdev and do things specific to virtio and then...

...tdev, dev_net(backup_netdev)); >+ SET_NETDEV_DEV(bypass_netdev, dev); >+ >+ bypass_netdev->netdev_ops = &virtnet_bypass_netdev_ops; >+ bypass_netdev->ethtool_ops = &virtnet_bypass_ethtool_ops; >+ >+ /* Initialize the device options */ >+ bypass_netdev->flags |= IFF_MASTER; I think I pointed that out already. Don't use "IFF_MASTER". That is specific to bonding. As I suggested in the reply to the patch #2, you should introduce IFF_BYPASS. Also, this flag should be set by the bypass module. Just create the netdev and do things specific to virtio and then...

...s_master_register() <snip> >>> >>>>>>>> + >>>>>>>> + /* Avoid Bonding master dev with same MAC registering as slave dev */ >>>>>>>> + if ((dev->priv_flags & IFF_BONDING) && (dev->flags & IFF_MASTER)) >>>>>>> Yeah, this is certainly incorrect. One thing is, you should be using the >>>>>>> helpers netif_is_bond_master(). >>>>>>> But what about the rest? macsec, macvlan, team, bridge, ovs and others? >>>>>>> >...

...>> >> > > > + return false; >> >> > > > + >> >> > > > + /* Avoid Bonding master dev with same MAC registering as slave dev */ >> >> > > > + if ((dev->priv_flags & IFF_BONDING) && (dev->flags & IFF_MASTER)) >> >> > > Yeah, this is certainly incorrect. One thing is, you should be using the >> >> > > helpers netif_is_bond_master(). >> >> > > But what about the rest? macsec, macvlan, team, bridge, ovs and others? >> >> > > >&gt...

...>> >> > > > + return false; >> >> > > > + >> >> > > > + /* Avoid Bonding master dev with same MAC registering as slave dev */ >> >> > > > + if ((dev->priv_flags & IFF_BONDING) && (dev->flags & IFF_MASTER)) >> >> > > Yeah, this is certainly incorrect. One thing is, you should be using the >> >> > > helpers netif_is_bond_master(). >> >> > > But what about the rest? macsec, macvlan, team, bridge, ovs and others? >> >> > > >&gt...

...NOTIFY_DONE; + + /* Avoid Vlan dev with same MAC registering as child dev */ + if (is_vlan_dev(event_dev)) + return NOTIFY_DONE; + + /* Avoid Bonding master dev with same MAC registering as child dev */ + if ((event_dev->priv_flags & IFF_BONDING) && + (event_dev->flags & IFF_MASTER)) + return NOTIFY_DONE; + + switch (event) { + case NETDEV_REGISTER: + return virtnet_bypass_register_child(event_dev); + case NETDEV_UNREGISTER: + return virtnet_bypass_unregister_child(event_dev); + case NETDEV_UP: + case NETDEV_DOWN: + case NETDEV_CHANGE: + return virtnet_bypass_update_link(...

...HER) - return NOTIFY_DONE; - - /* Avoid Vlan dev with same MAC registering as VF */ - if (is_vlan_dev(event_dev)) - return NOTIFY_DONE; - - /* Avoid Bonding master dev with same MAC registering as VF */ - if ((event_dev->priv_flags & IFF_BONDING) && - (event_dev->flags & IFF_MASTER)) - return NOTIFY_DONE; - - switch (event) { - case NETDEV_REGISTER: - return netvsc_register_vf(event_dev); - case NETDEV_UNREGISTER: - return netvsc_unregister_vf(event_dev); - case NETDEV_UP: - case NETDEV_DOWN: - return netvsc_vf_changed(event_dev); - default: - return NOTIFY_DONE; - } -}...

...HER) - return NOTIFY_DONE; - - /* Avoid Vlan dev with same MAC registering as VF */ - if (is_vlan_dev(event_dev)) - return NOTIFY_DONE; - - /* Avoid Bonding master dev with same MAC registering as VF */ - if ((event_dev->priv_flags & IFF_BONDING) && - (event_dev->flags & IFF_MASTER)) - return NOTIFY_DONE; - - switch (event) { - case NETDEV_REGISTER: - return netvsc_register_vf(event_dev); - case NETDEV_UNREGISTER: - return netvsc_unregister_vf(event_dev); - case NETDEV_UP: - case NETDEV_DOWN: - return netvsc_vf_changed(event_dev); - default: - return NOTIFY_DONE; - } -}...

...HER) - return NOTIFY_DONE; - - /* Avoid Vlan dev with same MAC registering as VF */ - if (is_vlan_dev(event_dev)) - return NOTIFY_DONE; - - /* Avoid Bonding master dev with same MAC registering as VF */ - if ((event_dev->priv_flags & IFF_BONDING) && - (event_dev->flags & IFF_MASTER)) - return NOTIFY_DONE; - - switch (event) { - case NETDEV_REGISTER: - return netvsc_register_vf(event_dev); - case NETDEV_UNREGISTER: - return netvsc_unregister_vf(event_dev); - case NETDEV_UP: - case NETDEV_DOWN: - return netvsc_vf_changed(event_dev); - default: - return NOTIFY_DONE; - } -}...

This patch series extends virtio_net to take over VF datapath by simulating a transparent bond without creating any additional netdev. I understand that there are some comments suggesting an alternate model that is based on 3 driver model(virtio_net, VF driver, a new driver virt_bond that acts as a master to virtio_net and VF). Would like to get some feedback on the right way to solve the live