From: Vladimir Oltean <vladimir.oltean@nxp•com>
To: "Alvin Šipraga" <ALSI@bang-olufsen•dk>
Cc: "netdev@vger•kernel.org" <netdev@vger•kernel.org>,
Florian Fainelli <f.fainelli@gmail•com>,
Andrew Lunn <andrew@lunn•ch>,
Vivien Didelot <vivien.didelot@gmail•com>,
Tobias Waldekranz <tobias@waldekranz•com>,
DENG Qingfang <dqfext@gmail•com>
Subject: Re: [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based
Date: Thu, 11 Nov 2021 12:45:28 +0000 [thread overview]
Message-ID: <20211111124528.2tecc3hoslheswl3@skbuf> (raw)
In-Reply-To: <a84ee210-b695-c352-8802-5b982d4037d5@bang-olufsen.dk>
On Thu, Nov 11, 2021 at 12:24:47PM +0000, Alvin Šipraga wrote:
> On 10/26/21 18:26, Vladimir Oltean wrote:
> > I have seen too many bugs already due to the fact that we must encode an
> > invalid dp->bridge_num as a negative value, because the natural tendency
> > is to check that invalid value using (!dp->bridge_num). Latest example
> > can be seen in commit 1bec0f05062c ("net: dsa: fix bridge_num not
> > getting cleared after ports leaving the bridge").
> >
> > Convert the existing users to assume that dp->bridge_num == 0 is the
> > encoding for invalid, and valid bridge numbers start from 1.
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp•com>
> > ---
>
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen•dk>
Thanks for the review.
> Small remark inline.
>
> > -int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
> > +unsigned int dsa_bridge_num_get(const struct net_device *bridge_dev, int max)
> > {
> > - int bridge_num = dsa_bridge_num_find(bridge_dev);
> > + unsigned int bridge_num = dsa_bridge_num_find(bridge_dev);
> >
> > - if (bridge_num < 0) {
> > + if (!bridge_num) {
> > /* First port that offloads TX forwarding for this bridge */
>
> Perhaps you want to update this comment in patch 2/6, since bridge_num
> is no longer just about TX forwarding offload.
>
> > - bridge_num = find_first_zero_bit(&dsa_fwd_offloading_bridges,
> > - DSA_MAX_NUM_OFFLOADING_BRIDGES);
> > + bridge_num = find_next_zero_bit(&dsa_fwd_offloading_bridges,
> > + DSA_MAX_NUM_OFFLOADING_BRIDGES,
> > + 1);
I will update this comment in patch 2 to say "First port that requests
FDB isolation or TX forwarding offload for this bridge". Sounds ok?
next prev parent reply other threads:[~2021-11-11 12:45 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 16:26 [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-10-26 16:26 ` [RFC PATCH net-next 1/6] net: dsa: make dp->bridge_num one-based Vladimir Oltean
2021-11-11 12:24 ` Alvin Šipraga
2021-11-11 12:45 ` Vladimir Oltean [this message]
2021-11-11 13:18 ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 2/6] net: dsa: assign a bridge number even without TX forwarding offload Vladimir Oltean
2021-11-11 12:27 ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 3/6] net: dsa: hide dp->bridge_dev and dp->bridge_num behind helpers Vladimir Oltean
2021-10-26 16:26 ` [RFC PATCH net-next 4/6] net: dsa: rename dsa_port_offloads_bridge to dsa_port_offloads_bridge_dev Vladimir Oltean
2021-11-11 12:33 ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 5/6] net: dsa: keep the bridge_dev and bridge_num as part of the same structure Vladimir Oltean
2021-11-11 13:17 ` Alvin Šipraga
2021-10-26 16:26 ` [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload Vladimir Oltean
2021-11-11 13:23 ` Alvin Šipraga
2021-11-19 15:38 ` Tobias Waldekranz
2021-11-02 10:07 ` [RFC PATCH net-next 0/6] Rework DSA bridge TX forwarding offload API Vladimir Oltean
2021-11-04 11:39 ` Tobias Waldekranz
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=20211111124528.2tecc3hoslheswl3@skbuf \
--to=vladimir.oltean@nxp$(echo .)com \
--cc=ALSI@bang-olufsen$(echo .)dk \
--cc=andrew@lunn$(echo .)ch \
--cc=dqfext@gmail$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tobias@waldekranz$(echo .)com \
--cc=vivien.didelot@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox