On Thu, Aug 26, 2021 at 9:06 PM Stephen Hemminger < step...@networkplumber.org> wrote:
> On Thu, 26 Aug 2021 20:46:47 +0300 > Igor Ryzhov <iryz...@nfware.com> wrote: > > > 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.
