All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>, Vladimir Oltean <olteanv@gmail.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	UNGLinuxDriver@microchip.com
Subject: Re: [PATCH v7 net-next 08/11] net: dsa: allow changing the tag protocol via the "tagging" device attribute
Date: Wed, 27 Jan 2021 17:51:00 -0800	[thread overview]
Message-ID: <06f6bbc8-c841-5071-ece5-4ee31adc7d36@gmail.com> (raw)
In-Reply-To: <20210127173044.65de6aba@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>



On 1/27/2021 5:30 PM, Jakub Kicinski wrote:
> On Tue, 26 Jan 2021 00:03:30 +0200 Vladimir Oltean wrote:
>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>
>> Currently DSA exposes the following sysfs:
>> $ cat /sys/class/net/eno2/dsa/tagging
>> ocelot
>>
>> which is a read-only device attribute, introduced in the kernel as
>> commit 98cdb4807123 ("net: dsa: Expose tagging protocol to user-space"),
>> and used by libpcap since its commit 993db3800d7d ("Add support for DSA
>> link-layer types").
>>
>> It would be nice if we could extend this device attribute by making it
>> writable:
>> $ echo ocelot-8021q > /sys/class/net/eno2/dsa/tagging
>>
>> This is useful with DSA switches that can make use of more than one
>> tagging protocol. It may be useful in dsa_loop in the future too, to
>> perform offline testing of various taggers, or for changing between dsa
>> and edsa on Marvell switches, if that is desirable.
>>
>> In terms of implementation, drivers can now move their tagging protocol
>> configuration outside of .setup/.teardown, and into .set_tag_protocol
>> and .del_tag_protocol. The calling order is:
>>
>> .setup -> [.set_tag_protocol -> .del_tag_protocol]+ -> .teardown
>>
>> There was one more contract between the DSA framework and drivers, which
>> is that if a CPU port needs to account for the tagger overhead in its
>> MTU configuration, it must do that privately. Which means it needs the
>> information about what tagger it uses before we call its MTU
>> configuration function. That promise is still held.
>>
>> Writing to the tagging sysfs will first tear down the tagging protocol
>> for all switches in the tree attached to that DSA master, then will
>> attempt setup with the new tagger.
>>
>> Writing will fail quickly with -EOPNOTSUPP for drivers that don't
>> support .set_tag_protocol, since that is checked during the deletion
>> phase. It is assumed that all switches within the same DSA tree use the
>> same driver, and therefore either all have .set_tag_protocol implemented,
>> or none do.
>>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
>> +const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf)
>> +{
>> +	const struct dsa_device_ops *ops = NULL;
>> +	struct dsa_tag_driver *dsa_tag_driver;
>> +
>> +	mutex_lock(&dsa_tag_drivers_lock);
>> +	list_for_each_entry(dsa_tag_driver, &dsa_tag_drivers_list, list) {
>> +		const struct dsa_device_ops *tmp = dsa_tag_driver->ops;
>> +
>> +		if (!sysfs_streq(buf, tmp->name))
>> +			continue;
>> +
>> +		ops = tmp;
>> +		break;
>> +	}
>> +	mutex_unlock(&dsa_tag_drivers_lock);
> 
> What's protecting from the tag driver unloading at this very moment?

Yes good point I missed that, too. The tag driver would need to get its
reference count incremented similarly to what dsa_tag_driver_get() does
and you would need to release the previous tag driver if successful in
grabbing the new one.
-- 
Florian

  reply	other threads:[~2021-01-28  1:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 22:03 [PATCH v7 net-next 00/11] tag_8021q for Ocelot switches Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 01/11] net: dsa: tag_8021q: add helpers to deduce whether a VLAN ID is RX or TX VLAN Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 02/11] net: mscc: ocelot: export VCAP structures to include/soc/mscc Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 03/11] net: mscc: ocelot: store a namespaced VCAP filter ID Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 04/11] net: mscc: ocelot: reapply bridge forwarding mask on bonding join/leave Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 05/11] net: mscc: ocelot: don't use NPI tag prefix for the CPU port module Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 06/11] net: dsa: document the existing switch tree notifiers and add a new one Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 07/11] net: dsa: keep a copy of the tagging protocol in the DSA switch tree Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 08/11] net: dsa: allow changing the tag protocol via the "tagging" device attribute Vladimir Oltean
2021-01-28  1:30   ` Jakub Kicinski
2021-01-28  1:51     ` Florian Fainelli [this message]
2021-01-28 20:07     ` Vladimir Oltean
2021-01-29  2:14       ` Jakub Kicinski
2021-01-25 22:03 ` [PATCH v7 net-next 09/11] net: dsa: felix: convert to the new .{set,del}_tag_protocol DSA API Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 10/11] net: dsa: add a second tagger for Ocelot switches based on tag_8021q Vladimir Oltean
2021-01-25 22:03 ` [PATCH v7 net-next 11/11] net: dsa: felix: perform switch setup for tag_8021q 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=06f6bbc8-c841-5071-ece5-4ee31adc7d36@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.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.