On Thu, Aug 26, 2021 at 9:06 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

On Thu, 26 Aug 2021 20:46:47 +0300
Igor Ryzhov [off-list ref] wrote:

quoted

Could you please clarify where exactly do I need to use rtnl lock?
From what I understand, netif_carrier_on/off can be called without the

lock.

Just a basic concern. The new stuff looks ok.
If you follow what current upstream virtio is doing, it should be safe.

See drivers/net/virtio/virtio_net.c, looks like more is needed?

Thanks for the suggestion, I looked at it.

Do you call ethtool_validate_speed() and ethtool_validate_duplex()?

The problem with those functions is that they are only available since
kernel version 4.6 and currently we have to also support 4.4. I don't think
we need to introduce more compatibility code, as nothing really bad
happens if the developer provides some invalued values. The worst
thing is that ethtool will show weird speed or unknown duplex.

Also, in virtio the driver does stop/wakeup of tx queues on carrier change.

This can be added for sure, but this improvement is not actually related to
my patch. We don't currently stop kernel's tx queue when the link is down
and I don't change that behavior. The code change is trivial but I'll need
to
find some time to test that everything works fine, so let's consider it a
separate thing, please.