-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Sunday, March 21, 2021 7:45 PM
To: Hariprasad Kelam <redacted>
Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org;
davem@davemloft.net; Sunil Kovvuri Goutham [off-list ref];
Linu Cherian [off-list ref]; Geethasowjanya Akula
[off-list ref]; Jerin Jacob Kollanukkaran [off-list ref];
Subbaraya Sundeep Bhatta [off-list ref]
Subject: [EXT] Re: [net-next PATCH 0/8] configuration support for switch
headers & phy
On Sun, Mar 21, 2021 at 05:39:50PM +0530, Hariprasad Kelam wrote:
quoted
This series of patches add support for parsing switch headers and
configuration support for phy modulation type(NRZ or PAM4).
PHYs that support changing modulation type ,user can configure it
through private flags pam4.
Marvell switches support DSA(distributed switch architecture) with
different switch headers like FDSA and EDSA. This patch series adds
private flags to enable user to configure interface in fdsa/edsa mode
such that flow steering (forwading packets to pf/vf depending on
switch header fields) and packet parsing can be acheived.
Hi Hariprasad
Private flags sound very wrong here. I would expect to see some integration
between the switchdev/DSA driver and the MAC driver.
Please show how this works in combination with drivers/net/dsa/mv88e6xxx
or drivers/net/ethernet/marvell/prestera.
Octeontx2 silicon supports NPC (network parser and cam) unit , through which packet parsing and packet classification is achieved.
Packet parsing extracting different fields from each layer.
DMAC + SMAC --> LA
VLAN ID --> LB
SIP + DIP --> LC
TCP SPORT + DPORT --> LD
And packet classification is achieved through flow identification in key extraction and mcam search key . User can install mcam rules
With action as
forward packet to PF and to receive queue 0
forward packet to VF and with as RSS ( Receive side scaling)
drop the packet
etc..
Now with switch header ( EDSA /FDSA) and HIGIG2 appended to regular packet , NPC can not parse these
Ingress packets as these headers does not have fixed headers. To achieve this Special PKIND( port kind) is allocated in hardware
which will help NPC to parse the packets.
For example incase of EDSA 8 byte header which is placed right after SMAC , special PKIND reserved for EDSA helps NPC to
Identify the input packet is EDSA . Such that NPC can extract fields in this header and forward to
Parse rest of the headers.
Same is the case with higig2 header where 16 bytes header is placed at start of the packet.
In this case private flags helps user to configure interface in EDSA/FDSA or HIGIG2. Such that special
PKIND reserved for that header are assigned to the interface. The scope of the patch series is how
User can configure interface mode as switch header(HIGIG2/EDSA etc) .In our case no DSA logical
Ports are created as these headers can be stripped by NPC.
Thanks,
Hariprasad k
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-03-23 20:27:28
On Tue, Mar 23, 2021 at 06:13:28PM +0000, Hariprasad Kelam wrote:
Hi Andrew ,
Please see inline,
No need to say that. That is the correct way to right emails.
quoted
Hi Hariprasad
Private flags sound very wrong here. I would expect to see some integration
between the switchdev/DSA driver and the MAC driver.
Please show how this works in combination with drivers/net/dsa/mv88e6xxx
or drivers/net/ethernet/marvell/prestera.
Octeontx2 silicon supports NPC (network parser and cam) unit , through which packet parsing and packet classification is achieved.
Packet parsing extracting different fields from each layer.
DMAC + SMAC --> LA
VLAN ID --> LB
SIP + DIP --> LC
TCP SPORT + DPORT --> LD
And packet classification is achieved through flow identification in key extraction and mcam search key . User can install mcam rules
With action as
forward packet to PF and to receive queue 0
forward packet to VF and with as RSS ( Receive side scaling)
drop the packet
etc..
Now with switch header ( EDSA /FDSA) and HIGIG2 appended to regular packet , NPC can not parse these
Ingress packets as these headers does not have fixed headers. To achieve this Special PKIND( port kind) is allocated in hardware
which will help NPC to parse the packets.
For example incase of EDSA 8 byte header which is placed right after SMAC , special PKIND reserved for EDSA helps NPC to
Identify the input packet is EDSA . Such that NPC can extract fields in this header and forward to
Parse rest of the headers.
Same is the case with higig2 header where 16 bytes header is placed at start of the packet.
In this case private flags helps user to configure interface in EDSA/FDSA or HIGIG2. Such that special
PKIND reserved for that header are assigned to the interface. The scope of the patch series is how
User can configure interface mode as switch header(HIGIG2/EDSA etc) .In our case no DSA logical
Ports are created as these headers can be stripped by NPC.
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
Hi Hariprasad
Private flags sound very wrong here. I would expect to see some integration
between the switchdev/DSA driver and the MAC driver.
Please show how this works in combination with drivers/net/dsa/mv88e6xxx
or drivers/net/ethernet/marvell/prestera.
Octeontx2 silicon supports NPC (network parser and cam) unit , through which packet parsing and packet classification is achieved.
Packet parsing extracting different fields from each layer.
DMAC + SMAC --> LA
VLAN ID --> LB
SIP + DIP --> LC
TCP SPORT + DPORT --> LD
And packet classification is achieved through flow identification in key extraction and mcam search key . User can install mcam rules
With action as
forward packet to PF and to receive queue 0
forward packet to VF and with as RSS ( Receive side scaling)
drop the packet
etc..
Now with switch header ( EDSA /FDSA) and HIGIG2 appended to regular packet , NPC can not parse these
Ingress packets as these headers does not have fixed headers. To achieve this Special PKIND( port kind) is allocated in hardware
which will help NPC to parse the packets.
For example incase of EDSA 8 byte header which is placed right after SMAC , special PKIND reserved for EDSA helps NPC to
Identify the input packet is EDSA . Such that NPC can extract fields in this header and forward to
Parse rest of the headers.
Same is the case with higig2 header where 16 bytes header is placed at start of the packet.
In this case private flags helps user to configure interface in EDSA/FDSA or HIGIG2. Such that special
PKIND reserved for that header are assigned to the interface. The scope of the patch series is how
User can configure interface mode as switch header(HIGIG2/EDSA etc) .In our case no DSA logical
Ports are created as these headers can be stripped by NPC.
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
What we are trying to do here has no dependency on DSA drivers and
neither impacts that functionality.
Here we are just notifying the HW to parse the packets properly.
Thanks,
Sunil.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-03-25 12:51:53
quoted
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
What we are trying to do here has no dependency on DSA drivers and
neither impacts that functionality.
So this is an indirect way of saying: Yes, this is for some out of
mainline Marvell SDK.
Here we are just notifying the HW to parse the packets properly.
But the correct way for this to happen is probably some kernel
internal API between the MAC and the DSA driver. Mainline probably has
no need for this private flag.
Andrew
On Thu, Mar 25, 2021 at 6:20 PM Andrew Lunn [off-list ref] wrote:
quoted
quoted
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
What we are trying to do here has no dependency on DSA drivers and
neither impacts that functionality.
So this is an indirect way of saying: Yes, this is for some out of
mainline Marvell SDK.
quoted
Here we are just notifying the HW to parse the packets properly.
But the correct way for this to happen is probably some kernel
internal API between the MAC and the DSA driver. Mainline probably has
no need for this private flag.
Andrew
Didn't get why you say so.
HW expects some info from SW to do the packet parsing properly and
this is specific to this hardware.
How can we generalize this ?
It's not just the DSA tags, the requirement is also for packets with
Higig header ie when system is connected to a switch
which appends Higig2 header to all pkts.
Thanks,
Sunil.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-03-25 13:22:10
On Thu, Mar 25, 2021 at 06:32:12PM +0530, Sunil Kovvuri wrote:
On Thu, Mar 25, 2021 at 6:20 PM Andrew Lunn [off-list ref] wrote:
quoted
quoted
quoted
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
What we are trying to do here has no dependency on DSA drivers and
neither impacts that functionality.
So this is an indirect way of saying: Yes, this is for some out of
mainline Marvell SDK.
quoted
Here we are just notifying the HW to parse the packets properly.
But the correct way for this to happen is probably some kernel
internal API between the MAC and the DSA driver. Mainline probably has
no need for this private flag.
Andrew
Didn't get why you say so.
HW expects some info from SW to do the packet parsing properly and
this is specific to this hardware.
Anything which needs out of mainline code is going to be rejected,
unless you can show there is an in mainline use case as well. Which is
why i keep pointing you to mv88e6xxx and prestira. That provides the
necessary {E}DSA tags. You have an mv88e6xxx and prestira switch being
controlled by Linux with DSA tagged frames flowing out of it into your
MAC driver. What is the big picture use case that requires this
private flag to enable DSA parsing? Why don't the MAC driver and the
DSA driver just talk to each other, and setup the parsing?
Andrew
On Thu, Mar 25, 2021 at 6:51 PM Andrew Lunn [off-list ref] wrote:
On Thu, Mar 25, 2021 at 06:32:12PM +0530, Sunil Kovvuri wrote:
quoted
On Thu, Mar 25, 2021 at 6:20 PM Andrew Lunn [off-list ref] wrote:
quoted
quoted
quoted
So you completely skipped how this works with mv88e6xxx or
prestera. If you need this private flag for some out of mainline
Marvell SDK, it is very unlikely to be accepted.
Andrew
What we are trying to do here has no dependency on DSA drivers and
neither impacts that functionality.
So this is an indirect way of saying: Yes, this is for some out of
mainline Marvell SDK.
quoted
Here we are just notifying the HW to parse the packets properly.
But the correct way for this to happen is probably some kernel
internal API between the MAC and the DSA driver. Mainline probably has
no need for this private flag.
Andrew
Didn't get why you say so.
HW expects some info from SW to do the packet parsing properly and
this is specific to this hardware.
Anything which needs out of mainline code is going to be rejected,
unless you can show there is an in mainline use case as well. Which is
why i keep pointing you to mv88e6xxx and prestira. That provides the
necessary {E}DSA tags. You have an mv88e6xxx and prestira switch being
controlled by Linux with DSA tagged frames flowing out of it into your
MAC driver. What is the big picture use case that requires this
private flag to enable DSA parsing? Why don't the MAC driver and the
DSA driver just talk to each other, and setup the parsing?
Andrew
The usecase is simple, unlike DSA tag, this 4byte FDSA tag doesn't
have a ethertype,
so HW cannot recognize this header. If such packers arise, then HW parsing will
fail and RSS will not work.
Hypothetically if we introduce some communication between MAC driver
and DSA driver,
wouldn't that also become specific to the device, what generic usecase
that communication
will have ?
Thanks,
Sunil.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-03-28 17:51:59
The usecase is simple, unlike DSA tag, this 4byte FDSA tag doesn't
have a ethertype,
so HW cannot recognize this header. If such packers arise, then HW parsing will
fail and RSS will not work.
Hypothetically if we introduce some communication between MAC driver
and DSA driver,
wouldn't that also become specific to the device, what generic usecase
that communication
will have ?
Hi Sunil
We need to be careful with wording. Due to history, the Linux kernel
uses dsa to mean any driver to control an Ethernet switch. It does not
imply the {E}DSA protocol used by Marvell switches, or even that the
switch is a Marvell switch.
netdev_uses_dsa(ndev) will tell you if the MAC is being used to
connect to a switch. It is set by the Linux DSA core when the switch
cluster is setup. That could be before or after the MAC is configured
up, which makes it a bit hard to use, since you don't have a clear
indicator when to evaluate to determine if you need to change your
packet parsing.
netdev_uses_dsa() looks at ndev->dsa_ptr. This is a pointer to the
structure which represents the port on the switch the MAC is connected
to. In Linux DSA terms, this is the CPU port. You can follow
dsa_ptr->tag_ops which gives you the tagger operations, i.e. those
used to add and remove the header/trailer. One member of that is
proto. This contains the tagging protocol, so EDSA, DSA, or
potentially FDSA, if that is ever supported. And this is all within
the core DSA code, so is generic. It should work for any tagging
protocol used by any switch which Linux DSA supports.
So actually, everything you need is already present, you don't need a
private flag. But adding a notifier that the MAC has been connected to
a switch and ndev->dsa_ptr is set would be useful. We could maybe use
NETDEV_CHANGE for that, or NETDEV_CHANGELOWERSTATE, since the MAC is
below the switch slave interfaces.
Andrew