On Sat, Jan 20, 2018 at 8:52 PM, John Fastabend
<john.fastab...@gmail.com> wrote:
> On 01/19/2018 03:09 PM, Cong Wang wrote:
>> This patch promotes the local change_tx_queue_len() to a core
>> helper function, dev_change_tx_queue_len(), so that rtnetlink
>> and net-sysfs could share the code. This also prepares for the
>> following patch.
>>
>> Note, the -EFAULT in the original code doesn't make sense,
>> we should propagate the errno from notifiers.
>>
>> Cc: John Fastabend <john.fastab...@gmail.com>
>> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
>> ---
>
>
> [...]
>
>> static ssize_t tx_queue_len_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t len)
>> @@ -355,7 +332,7 @@ static ssize_t tx_queue_len_store(struct device *dev,
>> if (!capable(CAP_NET_ADMIN))
>> return -EPERM;
>>
>> - return netdev_store(dev, attr, buf, len, change_tx_queue_len);
>> + return netdev_store(dev, attr, buf, len, dev_change_tx_queue_len);
>> }
>
> Is this protected by RTNL lock? If not what happens if this and do_setlink
> both try to change tx queue length at the same time? Seems we could get
> a race with multiple dev_deactivate/dev_activate sequences in-flight in
> the following 2/3 patch.
Surely it is protected by RTNL...
96 if (!rtnl_trylock())
97 return restart_syscall();
98
99 if (dev_isalive(netdev)) {
100 ret = (*set)(netdev, new);
101 if (ret == 0)
102 ret = len;
103 }
104 rtnl_unlock();
