Re: [PATCH net] hv_netvsc: preserve hw_features on mtu/channels/ringparam changes

2017-11-15 Thread Vitaly Kuznetsov 
Haiyang Zhang  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: Tuesday, November 14, 2017 11:58 AM
>> To: Stephen Hemminger 
>> Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org;
>> de...@linuxdriverproject.org; KY Srinivasan ; Haiyang
>> Zhang ; Stephen Hemminger
>> ; Mohammed Gamal 
>> Subject: Re: [PATCH net] hv_netvsc: preserve hw_features on
>> mtu/channels/ringparam changes
>> 
>> Stephen Hemminger  writes:
>> 
>> > On Tue, 14 Nov 2017 16:22:05 +0100
>> > Vitaly Kuznetsov  wrote:
>> >
>> > Yes, this looks like a real issue.
>> >
>> >> + /* Query hardware capabilities if we're called from netvsc_probe() */
>> >> + if (!net->hw_features) {
>> >> + ret = rndis_netdev_set_hwcaps(net_device, rndis_device);
>> >> + if (ret != 0)
>> >> + goto err_dev_remv;
>> >> + }
>> >> +
>> >
>> > Rather than conditional behavior in rndis_filter_device_add, it would
>> > be cleaner to make the call to get hardware capabilities there.
>> >
>> > Please respin and make the query of host a separate function.
>> 
>> You mean call rndis_netdev_set_hwcaps() from netvsc_probe()? Will do.
>> 
>> One question though: in case we'll be avoiding
>> rndis_filter_set_offload_params() call on mtu/channels/ringparam changes -
>> - can we trust the host to preserve what was there before the RNDIS reset?
>> In case not we'll have to untangle what is
>> rndis_netdev_set_hwcaps() in my patch splitting it into two: hw_features
>> setup and rndis_filter_set_offload_params() and leaving the later in
>> rndis_filter_device_add().
>
> After remove/re-add RNDIS dev, you should pass the parameters to the host
> again.
>

Thanks, this changes a lot. I'll prepare v2.

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH net] hv_netvsc: preserve hw_features on mtu/channels/ringparam changes

2017-11-14 Thread Haiyang Zhang 


> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Tuesday, November 14, 2017 11:58 AM
> To: Stephen Hemminger 
> Cc: net...@vger.kernel.org; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; KY Srinivasan ; Haiyang
> Zhang ; Stephen Hemminger
> ; Mohammed Gamal 
> Subject: Re: [PATCH net] hv_netvsc: preserve hw_features on
> mtu/channels/ringparam changes
> 
> Stephen Hemminger  writes:
> 
> > On Tue, 14 Nov 2017 16:22:05 +0100
> > Vitaly Kuznetsov  wrote:
> >
> > Yes, this looks like a real issue.
> >
> >> +  /* Query hardware capabilities if we're called from netvsc_probe() */
> >> +  if (!net->hw_features) {
> >> +  ret = rndis_netdev_set_hwcaps(net_device, rndis_device);
> >> +  if (ret != 0)
> >> +  goto err_dev_remv;
> >> +  }
> >> +
> >
> > Rather than conditional behavior in rndis_filter_device_add, it would
> > be cleaner to make the call to get hardware capabilities there.
> >
> > Please respin and make the query of host a separate function.
> 
> You mean call rndis_netdev_set_hwcaps() from netvsc_probe()? Will do.
> 
> One question though: in case we'll be avoiding
> rndis_filter_set_offload_params() call on mtu/channels/ringparam changes -
> - can we trust the host to preserve what was there before the RNDIS reset?
> In case not we'll have to untangle what is
> rndis_netdev_set_hwcaps() in my patch splitting it into two: hw_features
> setup and rndis_filter_set_offload_params() and leaving the later in
> rndis_filter_device_add().

After remove/re-add RNDIS dev, you should pass the parameters to the host
again.

Thanks,
- Haiyang
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net] hv_netvsc: preserve hw_features on mtu/channels/ringparam changes

2017-11-14 Thread Vitaly Kuznetsov 
Stephen Hemminger  writes:

> On Tue, 14 Nov 2017 16:22:05 +0100
> Vitaly Kuznetsov  wrote:
>
> Yes, this looks like a real issue.
>
>> +/* Query hardware capabilities if we're called from netvsc_probe() */
>> +if (!net->hw_features) {
>> +ret = rndis_netdev_set_hwcaps(net_device, rndis_device);
>> +if (ret != 0)
>> +goto err_dev_remv;
>> +}
>> +
>
> Rather than conditional behavior in rndis_filter_device_add, it would be 
> cleaner
> to make the call to get hardware capabilities there.
>
> Please respin and make the query of host a separate function.

You mean call rndis_netdev_set_hwcaps() from netvsc_probe()? Will do.

One question though: in case we'll be avoiding
rndis_filter_set_offload_params() call on mtu/channels/ringparam
changes -- can we trust the host to preserve what was there before the
RNDIS reset? In case not we'll have to untangle what is
rndis_netdev_set_hwcaps() in my patch splitting it into two: hw_features
setup and rndis_filter_set_offload_params() and leaving the later in
rndis_filter_device_add().

Thanks,

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net] hv_netvsc: preserve hw_features on mtu/channels/ringparam changes

2017-11-14 Thread Stephen Hemminger 
On Tue, 14 Nov 2017 16:22:05 +0100
Vitaly Kuznetsov  wrote:

Yes, this looks like a real issue.



> + /* Query hardware capabilities if we're called from netvsc_probe() */
> + if (!net->hw_features) {
> + ret = rndis_netdev_set_hwcaps(net_device, rndis_device);
> + if (ret != 0)
> + goto err_dev_remv;
> + }
> +

Rather than conditional behavior in rndis_filter_device_add, it would be cleaner
to make the call to get hardware capabilities there.

Please respin and make the query of host a separate function.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel