* [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
@ 2022-02-04 0:06 Mahesh Bandewar
2022-02-04 0:50 ` Jay Vosburgh
0 siblings, 1 reply; 7+ messages in thread
From: Mahesh Bandewar @ 2022-02-04 0:06 UTC (permalink / raw)
To: Netdev, Jay Vosburgh, Andy Gospodarek, Veaceslav Falico
Cc: David Miller, Jakub Kicinski, Mahesh Bandewar, Mahesh Bandewar
When 803.2ad mode enables a participating port, it should update
the slave-array. I have observed that the member links are participating
and are part of the active aggregator while the traffic is egressing via
only one member link (in a case where two links are participating). Via
krpobes I discovered that that slave-arr has only one link added while
the other participating link wasn't part of the slave-arr.
I couldn't see what caused that situation but the simple code-walk
through provided me hints that the enable_port wasn't always associated
with the slave-array update.
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
drivers/net/bonding/bond_3ad.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 6006c2e8fa2b..9fd1d6cba3cd 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1021,8 +1021,8 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
if (port->aggregator &&
port->aggregator->is_active &&
!__port_is_enabled(port)) {
-
__enable_port(port);
+ *update_slave_arr = true;
}
}
break;
@@ -1779,6 +1779,7 @@ static void ad_agg_selection_logic(struct aggregator *agg,
port = port->next_port_in_aggregator) {
__enable_port(port);
}
+ *update_slave_arr = true;
}
}
--
2.35.0.263.gb82422642f-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-04 0:06 [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates Mahesh Bandewar
@ 2022-02-04 0:50 ` Jay Vosburgh
2022-02-05 3:59 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Jay Vosburgh @ 2022-02-04 0:50 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Netdev, Andy Gospodarek, Veaceslav Falico, David Miller,
Jakub Kicinski, Mahesh Bandewar
Mahesh Bandewar <maheshb@google.com> wrote:
>When 803.2ad mode enables a participating port, it should update
>the slave-array. I have observed that the member links are participating
>and are part of the active aggregator while the traffic is egressing via
>only one member link (in a case where two links are participating). Via
>krpobes I discovered that that slave-arr has only one link added while
>the other participating link wasn't part of the slave-arr.
>
>I couldn't see what caused that situation but the simple code-walk
>through provided me hints that the enable_port wasn't always associated
>with the slave-array update.
>
>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>---
> drivers/net/bonding/bond_3ad.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 6006c2e8fa2b..9fd1d6cba3cd 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -1021,8 +1021,8 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> if (port->aggregator &&
> port->aggregator->is_active &&
> !__port_is_enabled(port)) {
>-
> __enable_port(port);
>+ *update_slave_arr = true;
> }
> }
> break;
>@@ -1779,6 +1779,7 @@ static void ad_agg_selection_logic(struct aggregator *agg,
> port = port->next_port_in_aggregator) {
> __enable_port(port);
> }
>+ *update_slave_arr = true;
> }
> }
>
>--
>2.35.0.263.gb82422642f-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-04 0:50 ` Jay Vosburgh
@ 2022-02-05 3:59 ` Jakub Kicinski
2022-02-07 5:52 ` Mahesh Bandewar (महेश बंडेवार)
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2022-02-05 3:59 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Mahesh Bandewar, Netdev, Andy Gospodarek, Veaceslav Falico,
David Miller, Mahesh Bandewar
On Thu, 03 Feb 2022 16:50:30 -0800 Jay Vosburgh wrote:
> Mahesh Bandewar <maheshb@google.com> wrote:
>
> >When 803.2ad mode enables a participating port, it should update
> >the slave-array. I have observed that the member links are participating
> >and are part of the active aggregator while the traffic is egressing via
> >only one member link (in a case where two links are participating). Via
> >krpobes I discovered that that slave-arr has only one link added while
kprobes
that that
The commit message would use some proof reading in general.
> >the other participating link wasn't part of the slave-arr.
> >
> >I couldn't see what caused that situation but the simple code-walk
> >through provided me hints that the enable_port wasn't always associated
> >with the slave-array update.
> >
> >Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Quacks like a fix, no? It's tagged for net-next and no fixes tag,
is there a reason why?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-05 3:59 ` Jakub Kicinski
@ 2022-02-07 5:52 ` Mahesh Bandewar (महेश बंडेवार)
2022-02-07 17:03 ` Jakub Kicinski
0 siblings, 1 reply; 7+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2022-02-07 5:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jay Vosburgh, Netdev, Andy Gospodarek, Veaceslav Falico,
David Miller, Mahesh Bandewar
On Fri, Feb 4, 2022 at 7:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 03 Feb 2022 16:50:30 -0800 Jay Vosburgh wrote:
> > Mahesh Bandewar <maheshb@google.com> wrote:
> >
> > >When 803.2ad mode enables a participating port, it should update
> > >the slave-array. I have observed that the member links are participating
> > >and are part of the active aggregator while the traffic is egressing via
> > >only one member link (in a case where two links are participating). Via
> > >krpobes I discovered that that slave-arr has only one link added while
>
> kprobes
> that that
>
> The commit message would use some proof reading in general.
>
:( will fix the typo and send it to you again.
> > >the other participating link wasn't part of the slave-arr.
> > >
> > >I couldn't see what caused that situation but the simple code-walk
> > >through provided me hints that the enable_port wasn't always associated
> > >with the slave-array update.
> > >
> > >Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> >
> > Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>
> Quacks like a fix, no? It's tagged for net-next and no fixes tag,
> is there a reason why?
Though this fixes some corner cases, I couldn't find anything obvious
that I can report as "fixes" hence decided otherwise. Does that make
sense?
thanks,
--mahesh..
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-07 5:52 ` Mahesh Bandewar (महेश बंडेवार)
@ 2022-02-07 17:03 ` Jakub Kicinski
2022-02-07 17:37 ` Jay Vosburgh
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2022-02-07 17:03 UTC (permalink / raw)
To: Mahesh Bandewar (महेश
बंडेवार)
Cc: Jay Vosburgh, Netdev, Andy Gospodarek, Veaceslav Falico,
David Miller, Mahesh Bandewar
On Sun, 6 Feb 2022 21:52:11 -0800 Mahesh Bandewar (महेश बंडेवार) wrote:
> On Fri, Feb 4, 2022 at 7:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Quacks like a fix, no? It's tagged for net-next and no fixes tag,
> > is there a reason why?
>
> Though this fixes some corner cases, I couldn't find anything obvious
> that I can report as "fixes" hence decided otherwise. Does that make
> sense?
So it's was not introduced in the refactorings which added
update_slave_arr? If the problem existed forever we can put:
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
it's just an indication how far back the backporting should go.
For anything older than oldest LTS (4.9) the exact tag probably
doesn't matter all that much.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-07 17:03 ` Jakub Kicinski
@ 2022-02-07 17:37 ` Jay Vosburgh
2022-02-07 22:18 ` Mahesh Bandewar (महेश बंडेवार)
0 siblings, 1 reply; 7+ messages in thread
From: Jay Vosburgh @ 2022-02-07 17:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Mahesh Bandewar, Netdev, Andy Gospodarek, Veaceslav Falico,
David Miller, Mahesh Bandewar
Jakub Kicinski <kuba@kernel.org> wrote:
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
>On Sun, 6 Feb 2022 21:52:11 -0800 Mahesh Bandewar (=E0=A4=AE=E0=A4=B9=E0=
=A5=87=E0=A4=B6 =E0=A4=AC=E0=A4=82=E0=A4=A1=E0=A5=87=E0=A4=B5=E0=A4=BE=E0=
=A4=B0) wrote:
>> On Fri, Feb 4, 2022 at 7:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
>> > Quacks like a fix, no? It's tagged for net-next and no fixes tag,
>> > is there a reason why?=20=20
>>=20
>> Though this fixes some corner cases, I couldn't find anything obvious
>> that I can report as "fixes" hence decided otherwise. Does that make
>> sense?
>
>So it's was not introduced in the refactorings which added
>update_slave_arr? If the problem existed forever we can put:
>
>Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
>it's just an indication how far back the backporting should go.
>For anything older than oldest LTS (4.9) the exact tag probably
>doesn't matter all that much.
I think the correct Fixes line would be the commit that
introduces the array logic in the first place, which I believe is:
Fixes: ee6377147409 ("bonding: Simplify the xmit function for modes that us=
e xmit_hash")
This dates to 3.18.
-J
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates
2022-02-07 17:37 ` Jay Vosburgh
@ 2022-02-07 22:18 ` Mahesh Bandewar (महेश बंडेवार)
0 siblings, 0 replies; 7+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2022-02-07 22:18 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Jakub Kicinski, Netdev, Andy Gospodarek, Veaceslav Falico,
David Miller, Mahesh Bandewar
On Mon, Feb 7, 2022 at 9:37 AM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Jakub Kicinski <kuba@kernel.org> wrote:
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: quoted-printable
>
> >On Sun, 6 Feb 2022 21:52:11 -0800 Mahesh Bandewar (=E0=A4=AE=E0=A4=B9=E0=
> =A5=87=E0=A4=B6 =E0=A4=AC=E0=A4=82=E0=A4=A1=E0=A5=87=E0=A4=B5=E0=A4=BE=E0=
> =A4=B0) wrote:
> >> On Fri, Feb 4, 2022 at 7:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >> > Quacks like a fix, no? It's tagged for net-next and no fixes tag,
> >> > is there a reason why?=20=20
> >>=20
> >> Though this fixes some corner cases, I couldn't find anything obvious
> >> that I can report as "fixes" hence decided otherwise. Does that make
> >> sense?
> >
> >So it's was not introduced in the refactorings which added
> >update_slave_arr? If the problem existed forever we can put:
> >
> >Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >
> >it's just an indication how far back the backporting should go.
> >For anything older than oldest LTS (4.9) the exact tag probably
> >doesn't matter all that much.
>
> I think the correct Fixes line would be the commit that
> introduces the array logic in the first place, which I believe is:
>
> Fixes: ee6377147409 ("bonding: Simplify the xmit function for modes that us=
> e xmit_hash")
>
> This dates to 3.18.
>
Will do. Thank you both.
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-07 22:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 0:06 [PATCH v3 net-next] bonding: pair enable_port with slave_arr_updates Mahesh Bandewar
2022-02-04 0:50 ` Jay Vosburgh
2022-02-05 3:59 ` Jakub Kicinski
2022-02-07 5:52 ` Mahesh Bandewar (महेश बंडेवार)
2022-02-07 17:03 ` Jakub Kicinski
2022-02-07 17:37 ` Jay Vosburgh
2022-02-07 22:18 ` Mahesh Bandewar (महेश बंडेवार)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).