On Fri, Jul 19, 2024 at 9:19 AM Michael S. Tsirkin [off-list ref] wrote:

On Fri, Jul 19, 2024 at 09:02:29AM +0800, Jason Wang wrote:

quoted

On Wed, Jul 17, 2024 at 2:53 PM Jason Wang [off-list ref] wrote:

quoted

On Wed, Jul 17, 2024 at 2:00 PM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Wed, Jul 17, 2024 at 09:19:02AM +0800, Jason Wang wrote:

quoted

On Wed, Jul 10, 2024 at 11:03 AM Jason Wang [off-list ref] wrote:

quoted

On Tue, Jul 9, 2024 at 9:28 PM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Tue, Jul 09, 2024 at 04:02:14PM +0800, Jason Wang wrote:

quoted

This patch synchronize operstate with admin state per RFC2863.

This is done by trying to toggle the carrier upon open/close and
synchronize with the config change work. This allows propagate status
correctly to stacked devices like:

ip link add link enp0s3 macvlan0 type macvlan
ip link set link enp0s3 down
ip link show

Before this patch:

3: enp0s3: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
......
5: macvlan0@enp0s3: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff

After this patch:

3: enp0s3: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 1000
    link/ether 00:00:05:00:00:09 brd ff:ff:ff:ff:ff:ff
...
5: macvlan0@enp0s3: <NO-CARRIER,BROADCAST,MULTICAST,UP,M-DOWN> mtu 1500 qdisc noqueue state LOWERLAYERDOWN mode DEFAULT group default qlen 1000
    link/ether b2:a9:c5:04:da:53 brd ff:ff:ff:ff:ff:ff

I think that the commit log is confusing. It seems to say that
the issue fixed is synchronizing state with hardware
config change.
But your example does not show any
hardware change. Isn't this example really just
a side effect of setting carrier off on close?

The main goal for this patch is to make virtio-net follow RFC2863. The
main thing that is missed is to synchronize the operstate with admin
state, if we do this, we get several good results, one of the obvious
one is to allow virtio-net to propagate status to the upper layer, for
example if the admin state of the lower virtio-net is down it should
be propagated to the macvlan on top, so I give the example of using a
stacked device. I'm not we had others but the commit log is probably
too small to say all of it.

Michael, any more comments on this?

Thans

Still don't get it, sorry.

quoted

quoted

quoted

quoted

This is done by trying to toggle the carrier upon open/close and
synchronize with the config change work.

What does this sentence mean? What is not synchronized with config
change that needs to be?

I meant,

1) maclvan depends on the linkwatch to transfer operstate from the
lower device to itself.
2) ndo_open()/close() will not trigger the linkwatch so we need to do
it by ourselves in virtio-net to make sure macvlan get the correct
opersate
3) consider config change work can change the state so ndo_close()
needs to synchronize with it

Thanks

Michael, are you fine with the above or I miss something there?

Thanks

I don't understand 3. config change can always trigger.
what I do not like is all these reads from config space
that now trigger on open/close. previously we did
read
- on probe
- after probe, if config changed


and that made sense.

Ok, not sure I get you all but I will post a new version to see.

Thanks

--
MST