public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Tobias Waldekranz <tobias@waldekranz•com>
To: Vladimir Oltean <olteanv@gmail•com>
Cc: davem@davemloft•net, kuba@kernel•org, andrew@lunn•ch,
	vivien.didelot@gmail•com, f.fainelli@gmail•com,
	j.vosburgh@gmail•com, vfalico@gmail•com, andy@greyhouse•net,
	netdev@vger•kernel.org
Subject: Re: [PATCH v4 net-next 3/5] net: dsa: Link aggregation support
Date: Wed, 16 Dec 2020 20:47:59 +0100	[thread overview]
Message-ID: <87pn39bki8.fsf@waldekranz.com> (raw)
In-Reply-To: <20201216162223.f7yndqvdlqt2akfs@skbuf>

On Wed, Dec 16, 2020 at 18:22, Vladimir Oltean <olteanv@gmail•com> wrote:
> On Wed, Dec 16, 2020 at 05:00:54PM +0100, Tobias Waldekranz wrote:
>> Monitor the following events and notify the driver when:
>> 
>> - A DSA port joins/leaves a LAG.
>> - A LAG, made up of DSA ports, joins/leaves a bridge.
>> - A DSA port in a LAG is enabled/disabled (enabled meaning
>>   "distributing" in 802.3ad LACP terms).
>> 
>> When a LAG joins a bridge, the DSA subsystem will treat that as each
>> individual port joining the bridge. The driver may look at the port's
>> LAG device pointer to see if it is associated with any LAG, if that is
>> required. This is analogue to how switchdev events are replicated out
>> to all lower devices when reaching e.g. a LAG.
>> 
>> Drivers can optionally request that DSA maintain a linear mapping from
>> a LAG ID to the corresponding netdev by setting ds->num_lag_ids to the
>> desired size.
>> 
>> In the event that the hardware is not capable of offloading a
>> particular LAG for any reason (the typical case being use of exotic
>> modes like broadcast), DSA will take a hands-off approach, allowing
>> the LAG to be formed as a pure software construct. This is reported
>> back through the extended ACK, but is otherwise transparent to the
>> user.
>> 
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz•com>
>> ---
>> 
>> I tried in vain to win checkpatch's approval of the foreach macros, no
>> matter how many parentheses I added. Looking at existing macros, this
>> style seems to be widely accepted. Is this a known issue?
>>
>>  include/net/dsa.h  | 60 +++++++++++++++++++++++++++++++++++
>>  net/dsa/dsa2.c     | 74 +++++++++++++++++++++++++++++++++++++++++++
>>  net/dsa/dsa_priv.h | 36 +++++++++++++++++++++
>>  net/dsa/port.c     | 79 ++++++++++++++++++++++++++++++++++++++++++++++
>>  net/dsa/slave.c    | 70 ++++++++++++++++++++++++++++++++++++----
>>  net/dsa/switch.c   | 50 +++++++++++++++++++++++++++++
>>  6 files changed, 362 insertions(+), 7 deletions(-)
>> 
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index 4e60d2610f20..9092c711a37c 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -149,8 +149,41 @@ struct dsa_switch_tree {
>>  
>>  	/* List of DSA links composing the routing table */
>>  	struct list_head rtable;
>> +
>> +	/* Maps offloaded LAG netdevs to a zero-based linear ID for
>> +	 * drivers that need it.
>> +	 */
>> +	struct net_device **lags;
>> +	unsigned int lags_len;
>>  };
>>  
>> +#define dsa_lags_foreach_id(_id, _dst)				\
>> +	for ((_id) = 0; (_id) < (_dst)->lags_len; (_id)++)	\
>> +		if ((_dst)->lags[_id])
>> +
>> +#define dsa_lag_foreach_port(_dp, _dst, _lag)	       \
>> +	list_for_each_entry(_dp, &(_dst)->ports, list) \
>> +		if ((_dp)->lag_dev == (_lag))
>
> ERROR: Macros with complex values should be enclosed in parentheses
> #86: FILE: include/net/dsa.h:160:
> +#define dsa_lags_foreach_id(_id, _dst)                         \
> +       for ((_id) = 0; (_id) < (_dst)->lags_len; (_id)++)      \
> +               if ((_dst)->lags[_id])
>                                  ~~~
>                                  missing parentheses
>
> ERROR: Macros with complex values should be enclosed in parentheses
> #90: FILE: include/net/dsa.h:164:
> +#define dsa_lag_foreach_port(_dp, _dst, _lag)         \
> +       list_for_each_entry(_dp, &(_dst)->ports, list) \
>                             ~~~
>                             missing parentheses
> +               if ((_dp)->lag_dev == (_lag))

Please see my comment just before the diffstat. I tried adding these
parentheses, and about a gagillion other ones. But no matter what I did
I could not make checkpatch happy.

This is where I gave up:

ERROR: Macros with complex values should be enclosed in parentheses
#160: FILE: include/net/dsa.h:160:
+#define dsa_lags_foreach_id(_id, _dst)                         \
+       for (((_id) = 0); ((_id) < ((_dst)->lags_len)); ((_id)++))      \
+               if ((_dst)->lags[(_id)])

ERROR: Macros with complex values should be enclosed in parentheses
#164: FILE: include/net/dsa.h:164:
+#define dsa_lag_foreach_port(_dp, _dst, _lag)         \
+       list_for_each_entry((_dp), &((_dst)->ports), list)      \
+               if (((_dp)->lag_dev) == (_lag))


  reply	other threads:[~2020-12-16 19:49 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 16:00 [PATCH v4 net-next 0/5] net: dsa: Link aggregation support Tobias Waldekranz
2020-12-16 16:00 ` [PATCH v4 net-next 1/5] net: bonding: Notify ports about their initial state Tobias Waldekranz
2020-12-16 16:28   ` Vladimir Oltean
2020-12-16 16:00 ` [PATCH v4 net-next 2/5] net: dsa: Don't offload port attributes on standalone ports Tobias Waldekranz
2020-12-16 16:27   ` Vladimir Oltean
2020-12-16 19:32     ` Tobias Waldekranz
2020-12-16 16:00 ` [PATCH v4 net-next 3/5] net: dsa: Link aggregation support Tobias Waldekranz
2020-12-16 16:22   ` Vladimir Oltean
2020-12-16 19:47     ` Tobias Waldekranz [this message]
2020-12-16 18:02   ` Vladimir Oltean
2020-12-16 19:49     ` Tobias Waldekranz
2020-12-16 18:44   ` Vladimir Oltean
2020-12-16 20:09     ` Tobias Waldekranz
2020-12-17 18:31       ` Vladimir Oltean
2020-12-16 16:00 ` [PATCH v4 net-next 4/5] net: dsa: mv88e6xxx: " Tobias Waldekranz
2020-12-16 19:04   ` Vladimir Oltean
2020-12-16 20:21     ` Tobias Waldekranz
2020-12-16 16:00 ` [PATCH v4 net-next 5/5] net: dsa: tag_dsa: Support reception of packets from LAG devices Tobias Waldekranz
2020-12-16 19:14   ` Vladimir Oltean

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=87pn39bki8.fsf@waldekranz.com \
    --to=tobias@waldekranz$(echo .)com \
    --cc=andrew@lunn$(echo .)ch \
    --cc=andy@greyhouse$(echo .)net \
    --cc=davem@davemloft$(echo .)net \
    --cc=f.fainelli@gmail$(echo .)com \
    --cc=j.vosburgh@gmail$(echo .)com \
    --cc=kuba@kernel$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=olteanv@gmail$(echo .)com \
    --cc=vfalico@gmail$(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