All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Jarod Wilson <jarod@redhat.com>
Cc: linux-kernel@vger.kernel.org,
	Veaceslav Falico <vfalico@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Thomas Davis <tadavis@lbl.gov>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next v2 4/4] bonding/balance-alb: put all slaves into promisc
Date: Fri, 21 May 2021 10:01:00 -0700	[thread overview]
Message-ID: <18879.1621616460@famine> (raw)
In-Reply-To: <20210521132756.1811620-5-jarod@redhat.com>

Jarod Wilson <jarod@redhat.com> wrote:

>Unlike most other modes with a primary interface, ALB mode bonding can
>receive on all slaves. That includes traffic destined for a non-local MAC
>behind a bridge on top of the bond. Such traffic gets dropped if the
>interface isn't in promiscuous mode. Therefore, it would seem to make
>sense to put all slaves into promisc.

	I'm still confused by this description and the actual changes of
the patch; the description above reads to me as you're intending that
all slaves of an alb bond should be promisc all the time, but the patch
below doesn't do that (it causes all alb mode slaves to be set to
promisc when the bond itself is set to promisc mode).  Could you
clarify?

	Also, your phrasing that "it would seem to make sense" suggests
to me that this change is speculative.  Do you have a specific example
of when the prior behavior causes an issue?

	-J

>Cc: Jay Vosburgh <j.vosburgh@gmail.com>
>Cc: Veaceslav Falico <vfalico@gmail.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Thomas Davis <tadavis@lbl.gov>
>Cc: netdev@vger.kernel.org
>Signed-off-by: Jarod Wilson <jarod@redhat.com>
>---
> drivers/net/bonding/bond_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 32785e9d0295..6d95f9e46059 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -647,9 +647,10 @@ static int bond_check_dev_link(struct bonding *bond,
> static int bond_set_promiscuity(struct bonding *bond, int inc)
> {
> 	struct list_head *iter;
>-	int err = 0;
>+	int mode, err = 0;
> 
>-	if (bond_uses_primary(bond)) {
>+	mode = BOND_MODE(bond);
>+	if (mode == BOND_MODE_ACTIVEBACKUP || mode == BOND_MODE_TLB) {
> 		struct slave *curr_active = rtnl_dereference(bond->curr_active_slave);
> 
> 		if (curr_active)
>-- 
>2.30.2


---
	-Jay Vosburgh, jay.vosburgh@canonical.com

      reply	other threads:[~2021-05-21 17:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 21:08 [PATCH 0/4] bond_alb: support VMs behind bridges better Jarod Wilson
2021-05-18 21:08 ` [PATCH 1/4] bonding: add pure source-mac-based tx hashing option Jarod Wilson
2021-05-19  9:01   ` Nikolay Aleksandrov
2021-05-18 21:08 ` [PATCH 2/4] bond_alb: don't rewrite bridged non-local MACs Jarod Wilson
2021-05-19 22:31   ` Jay Vosburgh
2021-05-18 21:08 ` [PATCH 3/4] bond_alb: don't tx balance multicast traffic either Jarod Wilson
2021-05-19 18:45   ` Jay Vosburgh
2021-05-18 21:08 ` [PATCH 4/4] bond_alb: put all slaves into promisc Jarod Wilson
2021-05-19 16:47   ` Jay Vosburgh
2021-05-21 13:27 ` [PATCH net-next v2 0/4] bonding/balance-alb: support VMs behind bridges better Jarod Wilson
2021-05-21 13:27   ` [PATCH net-next v2 1/4] bonding: add pure source-mac-based tx hashing option Jarod Wilson
2021-05-21 13:39     ` Nikolay Aleksandrov
2021-05-21 13:41       ` Nikolay Aleksandrov
2021-05-21 18:01       ` Jay Vosburgh
2021-05-21 13:27   ` [PATCH net-next v2 2/4] bonding/balance-lb: don't rewrite bridged non-local MACs Jarod Wilson
2021-05-21 17:58     ` Jay Vosburgh
2021-05-21 13:27   ` [PATCH net-next v2 3/4] bonding/balance-alb: don't tx balance multicast traffic either Jarod Wilson
2021-05-21 17:02     ` Jay Vosburgh
2021-05-21 13:27   ` [PATCH net-next v2 4/4] bonding/balance-alb: put all slaves into promisc Jarod Wilson
2021-05-21 17:01     ` Jay Vosburgh [this message]

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=18879.1621616460@famine \
    --to=jay.vosburgh@canonical.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=jarod@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tadavis@lbl.gov \
    --cc=vfalico@gmail.com \
    --subject='Re: [PATCH net-next v2 4/4] bonding/balance-alb: put all slaves into promisc' \
    /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

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.