mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: "Vivien Didelot" <vivien.didelot@savoirfairelinux.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel@savoirfairelinux.com,
	"David S. Miller" <davem@davemloft.net>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Uwe Kleine-König" <uwe@kleine-koenig.org>,
	"Andrey Smirnov" <andrew.smirnov@gmail.com>
Subject: Re: [PATCH net-next v2] net: dsa: make "label" property optional for dsa2
Date: Tue, 10 Jan 2017 09:58:18 -0800	[thread overview]
Message-ID: <3c7e5102-cef7-c909-a192-6238103e444e@gmail.com> (raw)
In-Reply-To: <20170110095524.GA1827@nanopsycho>

On 01/10/2017 01:55 AM, Jiri Pirko wrote:
> Mon, Jan 09, 2017 at 07:06:39PM CET, f.fainelli@gmail.com wrote:
>> On 01/09/2017 09:58 AM, Jiri Pirko wrote:
>>> Mon, Jan 09, 2017 at 06:42:07PM CET, f.fainelli@gmail.com wrote:
>>>> On 01/09/2017 08:06 AM, Jiri Pirko wrote:
>>>>> Mon, Jan 09, 2017 at 04:45:33PM CET, vivien.didelot@savoirfairelinux.com wrote:
>>>>>> Hi Jiri,
>>>>>>
>>>>>> Jiri Pirko <jiri@resnulli.us> writes:
>>>>>>
>>>>>>>> Extra question: shouldn't phys_port_{id,name} be switchdev attributes in
>>>>>>>
>>>>>>> Again, phys_port_id has nothing to do with switches. Should be removed
>>>>>>> from dsa because its use there is incorrect.
>>>>>>
>>>>>> Florian, since 3a543ef just got in, can it be reverted?
>>>>>
>>>>> Yes, please revert it. It is only in net-next.
>>>>
>>>> Maybe the use case can be understood before reverting the change. How do
>>>> we actually the physical port number of an Ethernet switch per-port
>>>> network device? The name is not enough, because there are plenty of
>>>> cases where we need to manipulate a physical port number (be it just for
>>>> informational purposes).
>>>
>>> Like what?
>>
>> Specifying the physical port number (and derive a queue number
>> eventually) for some ethtool (e.g: rxnfc)/tc (queue mapping) operations
>> where there is an action/queue/port destination argument that gets
>> programmed into the hardware.
> 
> Could you point me to a real example? User command?

ethtool --config-nfc moca flow-type udp4 src-ip 192.168.1.20 dst-ip \
        192.168.1.10 src-port 49884 dst-port 5001 action 2

Where 2 here designates a port number, users need to be able to look up
the physical port number corresponding to an interface to know which
value to put in this command.

Yes I know we can do the same thing with cls_flower, possibly by
referencing network devices directly.

> 
> 
>>
>> You already have the originating port number from the interface you call
>> the method against, but you also need the destination port number since
>> that is what the HW understands.
> 
> This is internal to kernel? I fail to understand what you mean exactly.

See the command above, from using the "moca" netdev here, we can access
the DSA private network device (dsa_slave_priv) structure and get the
port number from there, and pass this down to the switch driver. The
switch driver also takes another port number (and eventually a queue
number) to program classification filters.

> 
> 
>>
>> Aside from that, it is useful for allowing interface naming in user
>> space if you don't want to use labels.
>>
>>>
>>> Why the name is not enough? This is something propagated to userspace
>>> and never used internally in kernel.
>>
>> Because the name is not reflective of the port number in some switches.
>> In my case for instance, we have 5 ports that are named after the
>> entities they connect to (an integrated Gigabit PHY, two RGMII pads, one
>> MoCA interface, and the CPU)
>>
> 
> Again, I'm missing why you need a portnumber as a Integer to userspace.
> From driver, you can expose phys_port_name:

If we are exposing the port name here, we may as well expose the DSA
"label" instead of the physical port number number?

I don't deny my change may be misusing what phys_port_id was originally
designed for, but providing "p0" instead of "0" to user-space, what
value is there in adding the "p" in front really?
-- 
Florian

  reply	other threads:[~2017-01-10 17:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-08 23:15 Vivien Didelot
2017-01-08 23:30 ` Andrew Lunn
2017-01-09  2:56   ` Vivien Didelot
2017-01-09  7:32 ` Jiri Pirko
2017-01-09 15:04   ` Vivien Didelot
2017-01-09 15:11     ` Jiri Pirko
2017-01-09 15:45       ` Vivien Didelot
2017-01-09 16:00         ` Andrew Lunn
2017-01-09 16:07           ` Jiri Pirko
2017-01-09 16:06         ` Jiri Pirko
2017-01-09 17:42           ` Florian Fainelli
2017-01-09 17:58             ` Jiri Pirko
2017-01-09 18:06               ` Florian Fainelli
2017-01-10  9:55                 ` Jiri Pirko
2017-01-10 17:58                   ` Florian Fainelli [this message]
2017-01-11  7:26                     ` Jiri Pirko

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=3c7e5102-cef7-c909-a192-6238103e444e@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew.smirnov@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=kernel@savoirfairelinux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=uwe@kleine-koenig.org \
    --cc=vivien.didelot@savoirfairelinux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®