Linux Virtualization - May 2018 - [PATCH net-next v9 2/4] net: Introduce generic failover module

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",
>>+			   err);
>>+		goto err_handler_register;
>>+	}
>>+
>>+	err = netdev_upper_dev_link(slave_dev, failover_dev, NULL);
>
>Please use netdev_master_upper_dev_link().
Don't forget to fillup struct netdev_lag_upper_info -
NETDEV_LAG_TX_TYPE_ACTIVEBACKUP


Also, please call netdev_lower_state_changed() when the active slave
device changes from primary->backup of backup->primary and whenever link
state of a slave changes

[...]

On 5/2/2018 9:15 AM, Jiri Pirko wrote:
> 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",
>>> +			   err);
>>> +		goto err_handler_register;
>>> +	}
>>> +
>>> +	err = netdev_upper_dev_link(slave_dev, failover_dev, NULL);
>> Please use netdev_master_upper_dev_link().
> Don't forget to fillup struct netdev_lag_upper_info -
NETDEV_LAG_TX_TYPE_ACTIVEBACKUP
>
>
> Also, please call netdev_lower_state_changed() when the active slave
> device changes from primary->backup of backup->primary and whenever
link
> state of a slave 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?

On Wed, May 02, 2018 at 10:51:12AM -0700, Samudrala, Sridhar
wrote:
> 
> 
> On 5/2/2018 9:15 AM, Jiri Pirko wrote:
> > 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",
> > > > +			   err);
> > > > +		goto err_handler_register;
> > > > +	}
> > > > +
> > > > +	err = netdev_upper_dev_link(slave_dev, failover_dev,
NULL);
> > > Please use netdev_master_upper_dev_link().
> > Don't forget to fillup struct netdev_lag_upper_info -
NETDEV_LAG_TX_TYPE_ACTIVEBACKUP
> > 
> > 
> > Also, please call netdev_lower_state_changed() when the active slave
> > device changes from primary->backup of backup->primary and
whenever link
> > state of a slave 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.
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 slave
> devices for dhcp requests?
Try it?

Wed, May 02, 2018 at 07:51:12PM CEST, sridhar.samudrala at intel.com
wrote:
>
>
>On 5/2/2018 9:15 AM, Jiri Pirko wrote:
>> 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",
>> > > +			   err);
>> > > +		goto err_handler_register;
>> > > +	}
>> > > +
>> > > +	err = netdev_upper_dev_link(slave_dev, failover_dev, NULL);
>> > Please use netdev_master_upper_dev_link().
>> Don't forget to fillup struct netdev_lag_upper_info -
NETDEV_LAG_TX_TYPE_ACTIVEBACKUP
>> 
>> 
>> Also, please call netdev_lower_state_changed() when the active slave
>> device changes from primary->backup of backup->primary and
whenever link
>> state of a slave 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.
No. Don't set it. It is wrong.


>Does this help with the init scripts and network manager to skip slave
>devices for dhcp requests?
>

Wed, May 02, 2018 at 07:51:12PM CEST, sridhar.samudrala at intel.com
wrote:
>
>
>On 5/2/2018 9:15 AM, Jiri Pirko wrote:
>> 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",
>> > > +			   err);
>> > > +		goto err_handler_register;
>> > > +	}
>> > > +
>> > > +	err = netdev_upper_dev_link(slave_dev, failover_dev, NULL);
>> > Please use netdev_master_upper_dev_link().
>> Don't forget to fillup struct netdev_lag_upper_info -
NETDEV_LAG_TX_TYPE_ACTIVEBACKUP
>> 
>> 
>> Also, please call netdev_lower_state_changed() when the active slave
>> device changes from primary->backup of backup->primary and
whenever link
>> state of a slave 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.
I don't see any relation to that.