All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: linux-s390@vger.kernel.org,
	Alexandra Winter <wintera@linux.ibm.com>,
	netdev@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>,
	bridge@lists.linux-foundation.org,
	Ido Schimmel <idosch@nvidia.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Hangbin Liu <liuhangbin@gmail.com>,
	Roopa Prabhu <roopa@nvidia.com>, Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Pirko <jiri@nvidia.com>
Subject: Re: [Bridge] [PATCH net-next v2] veth: Support bonding events
Date: Wed, 30 Mar 2022 10:12:56 -0700	[thread overview]
Message-ID: <20220330101256.53f6ef48@kernel.org> (raw)
In-Reply-To: <c512e765-f411-9305-013b-471a07e7f3ff@blackwall.org>

On Wed, 30 Mar 2022 19:16:42 +0300 Nikolay Aleksandrov wrote:
> > Maybe opt-out? But assuming the event is only generated on
> > active/backup switch over - when would it be okay to ignore
> > the notification?
> 
> Let me just clarify, so I'm sure I've not misunderstood you. Do you mean opt-out as in
> make it default on? IMO that would be a problem, large scale setups would suddenly
> start propagating it to upper devices which would cause a lot of unnecessary bcast.
> I meant enable it only if needed, and only on specific ports (second part is not
> necessary, could be global, I think it's ok either way). I don't think any setup
> which has many upper vlans/macvlans would ever enable this.

That may be. I don't have a good understanding of scenarios in which
GARP is required and where it's not :) Goes without saying but the
default should follow the more common scenario.

> >> My concern was about the Hangbin's alternative proposal to notify all
> >> bridge ports. I hope in my porposal I was able to avoid infinite loops.  
> > 
> > Possibly I'm confused as to where the notification for bridge master
> > gets sent..  
> 
> IIUC it bypasses the bridge and sends a notify peers for the veth peer so it would
> generate a grat arp (inetdev_event -> NETDEV_NOTIFY_PEERS).

Ack, I was basically repeating the question of where does 
the notification with dev == br get generated.

There is a protection in this patch to make sure the other 
end of the veth is not plugged into a bridge (i.e. is not
a bridge port) but there can be a macvlan on top of that
veth that is part of a bridge, so IIUC that check is either
insufficient or unnecessary.

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: Alexandra Winter <wintera@linux.ibm.com>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	Hangbin Liu <liuhangbin@gmail.com>,
	netdev@vger.kernel.org, linux-s390@vger.kernel.org,
	Heiko Carstens <hca@linux.ibm.com>,
	Roopa Prabhu <roopa@nvidia.com>,
	bridge@lists.linux-foundation.org,
	Ido Schimmel <idosch@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>
Subject: Re: [PATCH net-next v2] veth: Support bonding events
Date: Wed, 30 Mar 2022 10:12:56 -0700	[thread overview]
Message-ID: <20220330101256.53f6ef48@kernel.org> (raw)
In-Reply-To: <c512e765-f411-9305-013b-471a07e7f3ff@blackwall.org>

On Wed, 30 Mar 2022 19:16:42 +0300 Nikolay Aleksandrov wrote:
> > Maybe opt-out? But assuming the event is only generated on
> > active/backup switch over - when would it be okay to ignore
> > the notification?
> 
> Let me just clarify, so I'm sure I've not misunderstood you. Do you mean opt-out as in
> make it default on? IMO that would be a problem, large scale setups would suddenly
> start propagating it to upper devices which would cause a lot of unnecessary bcast.
> I meant enable it only if needed, and only on specific ports (second part is not
> necessary, could be global, I think it's ok either way). I don't think any setup
> which has many upper vlans/macvlans would ever enable this.

That may be. I don't have a good understanding of scenarios in which
GARP is required and where it's not :) Goes without saying but the
default should follow the more common scenario.

> >> My concern was about the Hangbin's alternative proposal to notify all
> >> bridge ports. I hope in my porposal I was able to avoid infinite loops.  
> > 
> > Possibly I'm confused as to where the notification for bridge master
> > gets sent..  
> 
> IIUC it bypasses the bridge and sends a notify peers for the veth peer so it would
> generate a grat arp (inetdev_event -> NETDEV_NOTIFY_PEERS).

Ack, I was basically repeating the question of where does 
the notification with dev == br get generated.

There is a protection in this patch to make sure the other 
end of the veth is not plugged into a bridge (i.e. is not
a bridge port) but there can be a macvlan on top of that
veth that is part of a bridge, so IIUC that check is either
insufficient or unnecessary.

  reply	other threads:[~2022-03-30 17:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-29 11:40 [PATCH net-next v2] veth: Support bonding events Alexandra Winter
2022-03-30  0:54 ` [Bridge] " Jakub Kicinski
2022-03-30  0:54   ` Jakub Kicinski
2022-03-30 10:23   ` [Bridge] " Nikolay Aleksandrov
2022-03-30 10:23     ` Nikolay Aleksandrov
2022-03-30 11:14     ` [Bridge] " Alexandra Winter
2022-03-30 11:14       ` Alexandra Winter
2022-03-30 11:25       ` [Bridge] " Nikolay Aleksandrov
2022-03-30 11:25         ` Nikolay Aleksandrov
2022-03-30 15:51       ` [Bridge] " Jakub Kicinski
2022-03-30 15:51         ` Jakub Kicinski
2022-03-30 16:16         ` [Bridge] " Nikolay Aleksandrov
2022-03-30 16:16           ` Nikolay Aleksandrov
2022-03-30 17:12           ` Jakub Kicinski [this message]
2022-03-30 17:12             ` Jakub Kicinski
2022-03-30 19:15             ` [Bridge] " Jay Vosburgh
2022-03-30 19:15               ` Jay Vosburgh
2022-03-31  9:59               ` [Bridge] " Alexandra Winter
2022-03-31  9:59                 ` Alexandra Winter
2022-03-31 10:33                 ` [Bridge] " Nikolay Aleksandrov
2022-03-31 10:33                   ` Nikolay Aleksandrov
2022-03-31 12:07                   ` [Bridge] " Alexandra Winter
2022-03-31 12:07                     ` Alexandra Winter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220330101256.53f6ef48@kernel.org \
    --to=kuba@kernel.org \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=hca@linux.ibm.com \
    --cc=idosch@nvidia.com \
    --cc=j.vosburgh@gmail.com \
    --cc=jiri@nvidia.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=wintera@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.