From: Martin Havlik <hidden> Date: 2021-06-22 09:26:05
This patchset fixes the inability to use dedicated queues
on mlx5 PMD due to RTE Flow rule attempted creation prior to
starting the device.
Missing return value check and copy paste error near the rule
creation have also been fixed.
Cc: Chas Williams <redacted>
Cc: "Min Hu (Connor)" <redacted>
Cc: Declan Doherty <redacted>
Cc: Tomasz Kulasek <redacted>
Cc: Jan Viktorin <redacted>
Martin Havlik (3):
net/bonding: fix proper return value check and correct log message
net/bonding: fix not checked return value
net/bonding: start ethdev prior to setting 8023ad flow
drivers/net/bonding/rte_eth_bond_pmd.c | 33 +++++++++++++++++++-------
1 file changed, 25 insertions(+), 8 deletions(-)
--
2.27.0
From: Martin Havlik <hidden> Date: 2021-06-22 09:26:11
Return value is now saved to errval and log message on error
reports correct function name, doesn't use q_id which was out of context,
and uses up-to-date errval.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
From: Martin Havlik <hidden> Date: 2021-06-22 09:26:16
Return value from bond_ethdev_8023ad_flow_set() is now checked
and appropriate message is logged on error.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
From: Martin Havlik <hidden> Date: 2021-06-22 09:26:22
When dedicated queues are enabled, mlx5 PMD fails to install RTE Flows
if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set: port not started (slave_port=0 queue_id=1)
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Min Hu (Connor) <hidden> Date: 2021-06-23 07:05:02
在 2021/6/22 17:25, Martin Havlik 写道:
When dedicated queues are enabled, mlx5 PMD fails to install RTE Flows
if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set: port not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
quoted hunk
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Havlík Martin <hidden> Date: 2021-06-24 11:57:23
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install RTE Flows
if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set: port
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question.
Closest related info we found so far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and that
flow rules are assumed invalid after port stop/close/restart but doesn't
say anything about
<stop - flow rule create - start> vs <stop - start - flow rule create>
where the former is the point of failure on mlx5.
I'm addressing the maintainers for mlx5, who might know a bit more on
the topic.
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Andrew Rybchenko <hidden> Date: 2021-07-04 15:03:32
I guess review from net/mlx5 maintainers would be very useful
here. Adding in Cc.
On 6/22/21 12:25 PM, Martin Havlik wrote:
This patchset fixes the inability to use dedicated queues
on mlx5 PMD due to RTE Flow rule attempted creation prior to
starting the device.
Missing return value check and copy paste error near the rule
creation have also been fixed.
Cc: Chas Williams <redacted>
Cc: "Min Hu (Connor)" <redacted>
Cc: Declan Doherty <redacted>
Cc: Tomasz Kulasek <redacted>
Cc: Jan Viktorin <redacted>
Martin Havlik (3):
net/bonding: fix proper return value check and correct log message
net/bonding: fix not checked return value
net/bonding: start ethdev prior to setting 8023ad flow
drivers/net/bonding/rte_eth_bond_pmd.c | 33 +++++++++++++++++++-------
1 file changed, 25 insertions(+), 8 deletions(-)
When dedicated queues are enabled, mlx5 PMD fails to install RTE
Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set:
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered question.
Closest related info we found so far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and that flow rules are
assumed invalid after port stop/close/restart but doesn't say anything about
<stop - flow rule create - start> vs <stop - start - flow rule create> where the
former is the point of failure on mlx5.
I'm addressing the maintainers for mlx5, who might know a bit more on the
topic.
From rte_ethdev.h
* Please note that some configuration is not stored between calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
Mlx5 assumes flows are allowed to be configured only after rte_eth_dev_start().
Before start \ after stop - no flow is valid anymore.
Matan
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
When dedicated queues are enabled, mlx5 PMD fails to install RTE
Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set:
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and
that flow rules are assumed invalid after port stop/close/restart
but doesn't say anything about <stop - flow rule create - start> vs
<stop - start - flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for mlx5, who might
know a bit more on the topic.
Hello Matan,
From rte_ethdev.h
* Please note that some configuration is not stored between calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this
applies for all the quoted section, I think) but at least it can be used
as a counter argument.
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let the flows
be invalid after calling to rte_eth_dev_stop(). This is OK, flows must be
recreated and the bonding driver works this way. But why not *before start*?
Does somebody know how other drivers behaves in this situation? (We know and
can check for Intel, there it does not seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You may
want to start the port with thousands of flows that apply just when the
port starts (not after, that's late). This may introduce glitches in
filtering and measuring of traffic (well, it is a question how serious
issue could it be...).
This matters for the bonding case as well, doesn't it?. It is not
desirable to accidently omit a packet that was received by primary
ingress logic instead of being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert flow
rules before calling rte_eth_dev_start? Anyway, the behaviour should be
specified and documented in DPDK more precisely to avoid such
uncertainty in the future.
Jan
Matan
quoted
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Min Hu (Connor) <hidden> Date: 2021-07-08 12:33:28
The old code has just low-level error, which just copied the "printf"
above.
Thanks for your patch.
Acked-by: Min Hu (Connor) <redacted>
在 2021/6/22 17:25, Martin Havlik 写道:
quoted hunk
Return value is now saved to errval and log message on error
reports correct function name, doesn't use q_id which was out of context,
and uses up-to-date errval.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
From: Min Hu (Connor) <hidden> Date: 2021-07-08 12:43:57
在 2021/6/22 17:25, Martin Havlik 写道:
quoted hunk
Return value from bond_ethdev_8023ad_flow_set() is now checked
and appropriate message is logged on error.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Firstly, I think your patch is OK,
But I want to know what is your standard to decide which function should
check return value or not(of course, they are none-void)?
While, I noticed "bond_ethdev_lsc_event_callback" nearby was not
checked, but you ignored it.
From: Havlík Martin <hidden> Date: 2021-07-08 13:21:07
Dne 2021-07-08 14:43, Min Hu (Connor) napsal:
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
Return value from bond_ethdev_8023ad_flow_set() is now checked
and appropriate message is logged on error.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP
control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Firstly, I think your patch is OK,
But I want to know what is your standard to decide which function
should check return value or not(of course, they are none-void)?
I encountered the problem with dedicated queues on mlx5, I looked into
the source code and I saw the cause, so I fixed it. If I had seen any
other issue, I would've fixed it too. That, for example, applies to the
log message fix I included. My standard is to check all non-void return
values.
While, I noticed "bond_ethdev_lsc_event_callback" nearby was not
checked, but you ignored it.
Not ignored, just didn't properly review more code than what closely
surrounded the problematic lines.
From: Min Hu (Connor) <hidden> Date: 2021-07-09 00:09:13
在 2021/7/8 21:20, Havlík Martin 写道:
Dne 2021-07-08 14:43, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
Return value from bond_ethdev_8023ad_flow_set() is now checked
and appropriate message is logged on error.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP
control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Firstly, I think your patch is OK,
But I want to know what is your standard to decide which function
should check return value or not(of course, they are none-void)?
I encountered the problem with dedicated queues on mlx5, I looked into
the source code and I saw the cause, so I fixed it. If I had seen any
other issue, I would've fixed it too. That, for example, applies to the
log message fix I included. My standard is to check all non-void return
values.
Got it.
quoted
While, I noticed "bond_ethdev_lsc_event_callback" nearby was not
checked, but you ignored it.
Not ignored, just didn't properly review more code than what closely
surrounded the problematic lines.
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install RTE
Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and that
flow rules are assumed invalid after port stop/close/restart but
doesn't say anything about <stop - flow rule create - start> vs
<stop - start - flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for mlx5, who might
know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this applies for all
the quoted section, I think) but at least it can be used as a counter argument.
I agree the doc is not clear, as I see it flows are not part of configuration, at least not
when we are talking about rte_flow.
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let the flows be
invalid after calling to rte_eth_dev_stop(). This is OK, flows must be
recreated and the bonding driver works this way. But why not *before
start*?
Think about it this way by changing the configuration you may create invalid flows,
for example, you can only change the number of queues after port stop, so if
you create a flow with jump to queue 3 and then you remove queue 3 then,
the flow that is cached is not valid anymore. This goes for other configuration that
may affect the validity of a flow.
Does somebody know how other drivers behaves in this situation? (We know
and can check for Intel, there it does not seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You may want
to start the port with thousands of flows that apply just when the port starts
(not after, that's late). This may introduce glitches in filtering and measuring
of traffic (well, it is a question how serious issue could it be...).
Agree but this is always true nothing is done is zero time and even if it was the insertion
is not in zero time, (assuming that even if the flows are stored by the PMD until start
they still will not all be inserted in the same time)
This matters for the bonding case as well, doesn't it?. It is not desirable to
accidently omit a packet that was received by primary ingress logic instead of
being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert flow rules
before calling rte_eth_dev_start? Anyway, the behaviour should be specified
and documented in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
Ori
Jan
quoted
Matan
quoted
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Jan Viktorin <hidden> Date: 2021-07-11 21:45:41
On Sun, 11 Jul 2021 08:49:18 +0000
Ori Kam [off-list ref] wrote:
Hi Jan,
Hi Ori,
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install RTE
Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and that
flow rules are assumed invalid after port stop/close/restart but
doesn't say anything about <stop - flow rule create - start> vs
<stop - start - flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for mlx5, who might
know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this applies for all
the quoted section, I think) but at least it can be used as a counter argument.
I agree the doc is not clear, as I see it flows are not part of configuration, at least not
when we are talking about rte_flow.
Agree.
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let the flows be
invalid after calling to rte_eth_dev_stop(). This is OK, flows must be
recreated and the bonding driver works this way. But why not *before
start*?
Think about it this way by changing the configuration you may create invalid flows,
for example, you can only change the number of queues after port stop, so if
you create a flow with jump to queue 3 and then you remove queue 3 then,
the flow that is cached is not valid anymore. This goes for other configuration that
may affect the validity of a flow.
This is a valid argument, of course. The thing is whether it is
a responsibility of the PMD to take care of those corner cases or if
this is up to the application developer. If we respect the fact that
calling to stop invalidates all flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end
> port config rxq 3
> port start 0
it's clear that something is really wrong with the caller/user. I would
say that this is an application bug. I would expect that you first
reconfigure port and after that you modify flows. It seems quite
logical and intuitive, doesn't it?
Anyway, the PMD can possibly catch this rxq inconsistency but I can
imagine that there are more complicated sitations than just changing
count of queues. Any idea for a more complex real case?
Martin Havlik, can you please check how ixgbe and i40e behave in the
case above with "rxq change after flow created"?
quoted
Does somebody know how other drivers behaves in this situation? (We know
and can check for Intel, there it does not seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You may want
to start the port with thousands of flows that apply just when the port starts
(not after, that's late). This may introduce glitches in filtering and measuring
of traffic (well, it is a question how serious issue could it be...).
Agree but this is always true nothing is done is zero time and even if it was the insertion
is not in zero time, (assuming that even if the flows are stored by the PMD until start
they still will not all be inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow would
never be zero time, agree. But if I create flows before the port
starts, than I do not have to care too much about how long the creation
takes. Because the card is expected to be configured already before
pushing the "start button".
Maybe, you assume that the created flows would first be cached inside
the PMD and when the port is finally started, it than transparently
write the queues to the HW. But this is not what I was talking about,
that's maybe even worse because you are hiding such behaviour from
users...
(I do not know the exact mlx5 implementation details, so my answer can
miss something, sorry for that.)
quoted
This matters for the bonding case as well, doesn't it?. It is not desirable to
accidently omit a packet that was received by primary ingress logic instead of
being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert flow rules
before calling rte_eth_dev_start? Anyway, the behaviour should be specified
and documented in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
+1
Jan
Ori
quoted
Jan
quoted
Matan
quoted
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000
Ori Kam [off-list ref] wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install
RTE Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point
in https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and
that flow rules are assumed invalid after port
stop/close/restart but doesn't say anything about <stop - flow
rule create - start> vs <stop - start - flow rule create> where
the former is the point of failure on mlx5. I'm addressing the
maintainers for mlx5, who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between calls
to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be
re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this
applies for all the quoted section, I think) but at least it can be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not part of
configuration, at least not when we are talking about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let
the flows be invalid after calling to rte_eth_dev_stop(). This is
OK, flows must be recreated and the bonding driver works this way.
But why not *before start*?
Think about it this way by changing the configuration you may create
invalid flows, for example, you can only change the number of queues
after port stop, so if you create a flow with jump to queue 3 and then
you remove queue 3 then, the flow that is cached is not valid anymore.
This goes for other configuration that may affect the validity of a flow.
This is a valid argument, of course. The thing is whether it is a responsibility
of the PMD to take care of those corner cases or if this is up to the
application developer. If we respect the fact that calling to stop invalidates all
flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end > port config
rxq 3 > port start 0
it's clear that something is really wrong with the caller/user. I would say that
this is an application bug. I would expect that you first reconfigure port and
after that you modify flows. It seems quite logical and intuitive, doesn't it?
I agree the use case I described is bad programming, but the same logic can be
applied on stopping the port why flows should be deleted?
And even more important on some HW and some flows can only be inserted after
start since there is no real traffic gate, basically the start means adding the flows
if a flow is in the HW it is working, adding a gate will result in PPS degradation.
Anyway, the PMD can possibly catch this rxq inconsistency but I can imagine
that there are more complicated sitations than just changing count of
queues. Any idea for a more complex real case?
Some of the resource may be created only after start,
But I don't have a very good example now.
Martin Havlik, can you please check how ixgbe and i40e behave in the case
above with "rxq change after flow created"?
quoted
quoted
Does somebody know how other drivers behaves in this situation? (We
know and can check for Intel, there it does not seem to be an
issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You
may want to start the port with thousands of flows that apply just
when the port starts (not after, that's late). This may introduce
glitches in filtering and measuring of traffic (well, it is a question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time and even if
it was the insertion is not in zero time, (assuming that even if the
flows are stored by the PMD until start they still will not all be
inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow would never
be zero time, agree. But if I create flows before the port starts, than I do not
have to care too much about how long the creation takes. Because the card is
expected to be configured already before pushing the "start button".
Maybe, you assume that the created flows would first be cached inside the
PMD and when the port is finally started, it than transparently write the
queues to the HW. But this is not what I was talking about, that's maybe even
worse because you are hiding such behaviour from users...
(I do not know the exact mlx5 implementation details, so my answer can
miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of Nvidia we must
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the PMD needs to
translate this difference and in this gray area there is a game between
best API best performance understanding what the user really wants.
This is why the doc should be very clear on what is the expected.
but this is also the reason why such document is very hard to agree on.
Ori
quoted
quoted
This matters for the bonding case as well, doesn't it?. It is not
desirable to accidently omit a packet that was received by primary
ingress logic instead of being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert
flow rules before calling rte_eth_dev_start? Anyway, the behaviour
should be specified and documented in DPDK more precisely to avoid
such uncertainty in the future.
I agree the documentation should be fixed.
+1
Jan
quoted
Ori
quoted
Jan
quoted
Matan
quoted
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Havlík Martin <hidden> Date: 2021-07-13 08:18:27
Dne 2021-07-12 15:07, Ori Kam napsal:
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000
Ori Kam [off-list ref] wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install
RTE Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point
in https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and
that flow rules are assumed invalid after port
stop/close/restart but doesn't say anything about <stop - flow
rule create - start> vs <stop - start - flow rule create> where
the former is the point of failure on mlx5. I'm addressing the
maintainers for mlx5, who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between calls
to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode, all-multicast
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be
re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this
applies for all the quoted section, I think) but at least it can be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not part of
configuration, at least not when we are talking about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let
the flows be invalid after calling to rte_eth_dev_stop(). This is
OK, flows must be recreated and the bonding driver works this way.
But why not *before start*?
Think about it this way by changing the configuration you may create
invalid flows, for example, you can only change the number of queues
after port stop, so if you create a flow with jump to queue 3 and then
you remove queue 3 then, the flow that is cached is not valid anymore.
This goes for other configuration that may affect the validity of a flow.
This is a valid argument, of course. The thing is whether it is a
responsibility
of the PMD to take care of those corner cases or if this is up to the
application developer. If we respect the fact that calling to stop
invalidates all
flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end >
port config
rxq 3 > port start 0
it's clear that something is really wrong with the caller/user. I
would say that
this is an application bug. I would expect that you first reconfigure
port and
after that you modify flows. It seems quite logical and intuitive,
doesn't it?
I agree the use case I described is bad programming, but the same logic
can be
applied on stopping the port why flows should be deleted?
And even more important on some HW and some flows can only be inserted
after
start since there is no real traffic gate, basically the start means
adding the flows
if a flow is in the HW it is working, adding a gate will result in PPS
degradation.
quoted
Anyway, the PMD can possibly catch this rxq inconsistency but I can
imagine
that there are more complicated sitations than just changing count of
queues. Any idea for a more complex real case?
Some of the resource may be created only after start,
But I don't have a very good example now.
quoted
Martin Havlik, can you please check how ixgbe and i40e behave in the
case
above with "rxq change after flow created"?
Both PMDs do not raise any errors in the mentioned case. The packets
that should go to the non-existent queue are dropped at some point in
the processing (from what I could see and understand).
I'm glad for the discussion that is taking place but I feel we have
strayed from the original reason of it. That being the inability to
enable dedicated queues for bonding mode 8023AD on mlx5. We have
concluded that flow creation has to be done after port/device start. So
moving the function call to create the flow after device start should
solve the issue
(https://mails.dpdk.org/archives/dev/2021-June/212210.html). From my
testing, flow create after start is not a problem on Intel PMDs.
I'm turning to you, Connor, as you have made the original question on
this. Is the patch I presented applicable?
Martin
quoted
quoted
quoted
Does somebody know how other drivers behaves in this situation? (We
know and can check for Intel, there it does not seem to be an
issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You
may want to start the port with thousands of flows that apply just
when the port starts (not after, that's late). This may introduce
glitches in filtering and measuring of traffic (well, it is a question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time and even if
it was the insertion is not in zero time, (assuming that even if the
flows are stored by the PMD until start they still will not all be
inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow
would never
be zero time, agree. But if I create flows before the port starts,
than I do not
have to care too much about how long the creation takes. Because the
card is
expected to be configured already before pushing the "start button".
Maybe, you assume that the created flows would first be cached inside
the
PMD and when the port is finally started, it than transparently write
the
queues to the HW. But this is not what I was talking about, that's
maybe even
worse because you are hiding such behaviour from users...
(I do not know the exact mlx5 implementation details, so my answer can
miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of Nvidia
we must
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the PMD needs
to
translate this difference and in this gray area there is a game between
best API best performance understanding what the user really wants.
This is why the doc should be very clear on what is the expected.
but this is also the reason why such document is very hard to agree on.
Ori
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?. It is not
desirable to accidently omit a packet that was received by primary
ingress logic instead of being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert
flow rules before calling rte_eth_dev_start? Anyway, the behaviour
should be specified and documented in DPDK more precisely to avoid
such uncertainty in the future.
I agree the documentation should be fixed.
+1
Jan
quoted
Ori
quoted
Jan
quoted
Matan
quoted
quoted
quoted
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 26
++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
From: Andrew Rybchenko <hidden> Date: 2021-07-13 09:26:40
On 7/13/21 11:18 AM, Havlík Martin wrote:
Dne 2021-07-12 15:07, Ori Kam napsal:
quoted
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000
Ori Kam [off-list ref] wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to install
RTE Flows if the underlying ethdev is not started:
bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5 centered
question. Closest related info we found so far is the 5th point
in https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and
that flow rules are assumed invalid after port
stop/close/restart but doesn't say anything about <stop - flow
rule create - start> vs <stop - start - flow rule create> where
the former is the point of failure on mlx5. I'm addressing the
maintainers for mlx5, who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between calls
to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode,
all-multicast
quoted
quoted
quoted
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to be
re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and this
applies for all the quoted section, I think) but at least it can
be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not part of
configuration, at least not when we are talking about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is valid
anymore.
I believe that this discussion is not about validity of flows. Let
the flows be invalid after calling to rte_eth_dev_stop(). This is
OK, flows must be recreated and the bonding driver works this way.
But why not *before start*?
Think about it this way by changing the configuration you may create
invalid flows, for example, you can only change the number of queues
after port stop, so if you create a flow with jump to queue 3 and then
you remove queue 3 then, the flow that is cached is not valid anymore.
This goes for other configuration that may affect the validity of a
flow.
This is a valid argument, of course. The thing is whether it is a
responsibility
of the PMD to take care of those corner cases or if this is up to the
application developer. If we respect the fact that calling to stop
invalidates all
flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end >
port config
rxq 3 > port start 0
it's clear that something is really wrong with the caller/user. I
would say that
this is an application bug. I would expect that you first reconfigure
port and
after that you modify flows. It seems quite logical and intuitive,
doesn't it?
I agree the use case I described is bad programming, but the same
logic can be
applied on stopping the port why flows should be deleted?
And even more important on some HW and some flows can only be inserted
after
start since there is no real traffic gate, basically the start means
adding the flows
if a flow is in the HW it is working, adding a gate will result in PPS
degradation.
quoted
Anyway, the PMD can possibly catch this rxq inconsistency but I can
imagine
that there are more complicated sitations than just changing count of
queues. Any idea for a more complex real case?
Some of the resource may be created only after start,
But I don't have a very good example now.
quoted
Martin Havlik, can you please check how ixgbe and i40e behave in the
case
above with "rxq change after flow created"?
Both PMDs do not raise any errors in the mentioned case. The packets
that should go to the non-existent queue are dropped at some point in
the processing (from what I could see and understand).
I'm glad for the discussion that is taking place but I feel we have
strayed from the original reason of it. That being the inability to
enable dedicated queues for bonding mode 8023AD on mlx5. We have
concluded that flow creation has to be done after port/device start. So
moving the function call to create the flow after device start should
solve the issue
(https://mails.dpdk.org/archives/dev/2021-June/212210.html). From my
testing, flow create after start is not a problem on Intel PMDs.
It would be good to hear what net/bonding maintainers think
about it. I see no conclusion.
If net/mlx5 behaviour is fixed, will it fix the initial
problem?
I'm turning to you, Connor, as you have made the original question on
this. Is the patch I presented applicable?
Martin
quoted
quoted
quoted
quoted
Does somebody know how other drivers behaves in this situation? (We
know and can check for Intel, there it does not seem to be an
issue.)
By the way, the mlx5 behaviour opens a (probably short) time window
between starting of a port and configuation of filtering flows. You
may want to start the port with thousands of flows that apply just
when the port starts (not after, that's late). This may introduce
glitches in filtering and measuring of traffic (well, it is a
question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time and even if
it was the insertion is not in zero time, (assuming that even if the
flows are stored by the PMD until start they still will not all be
inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow
would never
be zero time, agree. But if I create flows before the port starts,
than I do not
have to care too much about how long the creation takes. Because the
card is
expected to be configured already before pushing the "start button".
Maybe, you assume that the created flows would first be cached inside
the
PMD and when the port is finally started, it than transparently write
the
queues to the HW. But this is not what I was talking about, that's
maybe even
worse because you are hiding such behaviour from users...
(I do not know the exact mlx5 implementation details, so my answer can
miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of Nvidia
we must
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the PMD
needs to
translate this difference and in this gray area there is a game between
best API best performance understanding what the user really wants.
This is why the doc should be very clear on what is the expected.
but this is also the reason why such document is very hard to agree on.
Do I understand correctly that net/mlx5 maintainers will follow
up?
quoted
Ori
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?. It is not
desirable to accidently omit a packet that was received by primary
ingress logic instead of being redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible to insert
flow rules before calling rte_eth_dev_start? Anyway, the behaviour
should be specified and documented in DPDK more precisely to avoid
such uncertainty in the future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to
insert flows before pushing start button which enables the
traffic if HW supports it).
So, I'm applying first two patches and hold on this one.
From: Andrew Rybchenko <hidden> Date: 2021-07-13 09:26:46
On 7/9/21 3:09 AM, Min Hu (Connor) wrote:
在 2021/7/8 21:20, Havlík Martin 写道:
quoted
Dne 2021-07-08 14:43, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
Return value from bond_ethdev_8023ad_flow_set() is now checked
and appropriate message is logged on error.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP
control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Firstly, I think your patch is OK,
But I want to know what is your standard to decide which function
should check return value or not(of course, they are none-void)?
I encountered the problem with dedicated queues on mlx5, I looked into
the source code and I saw the cause, so I fixed it. If I had seen any
other issue, I would've fixed it too. That, for example, applies to
the log message fix I included. My standard is to check all non-void
return values.
Got it.
quoted
quoted
While, I noticed "bond_ethdev_lsc_event_callback" nearby was not
checked, but you ignored it.
Not ignored, just didn't properly review more code than what closely
surrounded the problematic lines.
quoted
quoted
/* Start device */
Acked by: Min Hu (Connor) [off-list ref]
quoted
.
Acked-by: Min Hu (Connor) <redacted>
Reviewed-by: Andrew Rybchenko <redacted>
Applied, thanks.
From: Andrew Rybchenko <hidden> Date: 2021-07-13 09:26:52
On 7/8/21 3:33 PM, Min Hu (Connor) wrote:
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
Return value is now saved to errval and log message on error
reports correct function name, doesn't use q_id which was out of context,
and uses up-to-date errval.
Fixes: 112891cd27e5 ("net/bonding: add dedicated HW queues for LACP
control")
Cc: tomaszx.kulasek@intel.com
Signed-off-by: Martin Havlik <redacted>
Cc: Jan Viktorin <redacted>
Acked-by: Min Hu (Connor) <redacted>
Reviewed-by: Andrew Rybchenko <redacted>
Applied, thanks.
From: Jan Viktorin <hidden> Date: 2021-07-13 11:06:13
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
On 7/13/21 11:18 AM, Havlík Martin wrote:
quoted
Dne 2021-07-12 15:07, Ori Kam napsal:
quoted
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000
Ori Kam [off-list ref] wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000
Matan Azrad [off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to
install RTE Flows if the underlying ethdev is not
started: bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5
centered question. Closest related info we found so far
is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's
responsibility and that flow rules are assumed invalid
after port stop/close/restart but doesn't say anything
about <stop - flow rule create - start> vs <stop - start
- flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for mlx5,
who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between
calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode,
all-multicast
quoted
quoted
quoted
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering
rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need
to be
re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows
(and this applies for all the quoted section, I think) but at
least it can
be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not part of
configuration, at least not when we are talking about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is
valid anymore.
I believe that this discussion is not about validity of
flows. Let the flows be invalid after calling to
rte_eth_dev_stop(). This is OK, flows must be recreated and
the bonding driver works this way. But why not *before
start*?
Think about it this way by changing the configuration you may
create invalid flows, for example, you can only change the
number of queues after port stop, so if you create a flow with
jump to queue 3 and then you remove queue 3 then, the flow that
is cached is not valid anymore. This goes for other
configuration that may affect the validity of a
flow.
This is a valid argument, of course. The thing is whether it is a
responsibility
of the PMD to take care of those corner cases or if this is up to
the application developer. If we respect the fact that calling to
stop invalidates all
flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end
quoted
port config
rxq 3 > port start 0
it's clear that something is really wrong with the caller/user. I
would say that
this is an application bug. I would expect that you first
reconfigure port and
after that you modify flows. It seems quite logical and intuitive,
doesn't it?
I agree the use case I described is bad programming, but the same
logic can be
applied on stopping the port why flows should be deleted?
I just took into account what Matan has written. Certainly, it is not
necessary to delete flows on stop.
quoted
quoted
And even more important on some HW and some flows can only be
inserted after
start since there is no real traffic gate, basically the start
means adding the flows
if a flow is in the HW it is working, adding a gate will result in
PPS degradation.
What do you mean by term "gate", here? Some FPGA logic? Or something
else? Why "adding a gate" would result in PPS degradation? Can you
make those ideas a bit clearer?
quoted
quoted
quoted
Anyway, the PMD can possibly catch this rxq inconsistency but I
can imagine
that there are more complicated sitations than just changing
count of queues. Any idea for a more complex real case?
Some of the resource may be created only after start,
But I don't have a very good example now.
Please, try to find some reasonable example otherwise your argumentation
does not make much sense...
quoted
quoted
quoted
Martin Havlik, can you please check how ixgbe and i40e behave in
the case
above with "rxq change after flow created"?
Both PMDs do not raise any errors in the mentioned case. The packets
that should go to the non-existent queue are dropped at some point
in the processing (from what I could see and understand).
Thanks for clarification. It would be nice if any other PMD can be
tested for this behaviour. Can sombody help us with other NICs?
quoted
I'm glad for the discussion that is taking place but I feel we have
strayed from the original reason of it. That being the inability to
enable dedicated queues for bonding mode 8023AD on mlx5. We have
concluded that flow creation has to be done after port/device
start. So moving the function call to create the flow after device
start should solve the issue
(https://mails.dpdk.org/archives/dev/2021-June/212210.html). From my
testing, flow create after start is not a problem on Intel PMDs.
It would be good to hear what net/bonding maintainers think
about it. I see no conclusion.
If net/mlx5 behaviour is fixed, will it fix the initial
problem?
I believe that if mlx5 allows to create flows before calling to
rte_eth_dev_start() then the bonding would work as expected.
quoted
I'm turning to you, Connor, as you have made the original question
on this. Is the patch I presented applicable?
Martin
quoted
quoted
quoted
quoted
Does somebody know how other drivers behaves in this
situation? (We know and can check for Intel, there it does
not seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short) time
window between starting of a port and configuation of
filtering flows. You may want to start the port with
thousands of flows that apply just when the port starts (not
after, that's late). This may introduce glitches in filtering
and measuring of traffic (well, it is a
question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time and
even if it was the insertion is not in zero time, (assuming
that even if the flows are stored by the PMD until start they
still will not all be inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow
would never
be zero time, agree. But if I create flows before the port starts,
than I do not
have to care too much about how long the creation takes. Because
the card is
expected to be configured already before pushing the "start
button".
Maybe, you assume that the created flows would first be cached
inside the
PMD and when the port is finally started, it than transparently
write the
queues to the HW. But this is not what I was talking about, that's
maybe even
worse because you are hiding such behaviour from users...
(I do not know the exact mlx5 implementation details, so my
answer can miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of
Nvidia we must
By "must" you probably mean "would have to"? Is that correct?
quoted
quoted
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the PMD
needs to
translate this difference and in this gray area there is a game
between best API best performance understanding what the user
Yes, as a former FPGA dev, I can understand your concerns. RTE Flow is
a difficult beast.
Jan
quoted
quoted
really wants. This is why the doc should be very clear on what is
the expected. but this is also the reason why such document is
very hard to agree on.
Do I understand correctly that net/mlx5 maintainers will follow
up?
quoted
quoted
Ori
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?. It is
not desirable to accidently omit a packet that was received
by primary ingress logic instead of being redirected into the
dedicated queue.
Are there any chances that for mlx5 it would be possible to
insert flow rules before calling rte_eth_dev_start? Anyway,
the behaviour should be specified and documented in DPDK more
precisely to avoid such uncertainty in the future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to
insert flows before pushing start button which enables the
traffic if HW supports it).
So, I'm applying first two patches and hold on this one.
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Tuesday, July 13, 2021 2:06 PM
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
quoted
On 7/13/21 11:18 AM, Havlík Martin wrote:
quoted
Dne 2021-07-12 15:07, Ori Kam napsal:
quoted
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000 Ori Kam [off-list ref]
wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000 Matan Azrad
[off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails to
install RTE Flows if the underlying ethdev is not
started: bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port started ?
I noticed other PMDs did not has that reliance.
After looking into it, I really can't answer this mlx5
centered question. Closest related info we found so far is
the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#cavea
ts but it only specifies it's the application's
responsibility and that flow rules are assumed invalid
after port stop/close/restart but doesn't say anything
about <stop - flow rule create - start> vs <stop - start
- flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for mlx5,
who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored between
calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode,
all-multicast
quoted
quoted
quoted
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering
rules)
* - NIC queue statistics mappings
just after this section, you can find the following statement:
* Any other configuration will not be stored and will need to
be re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows (and
this applies for all the quoted section, I think) but at least
it can
be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not part of
configuration, at least not when we are talking about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only after
rte_eth_dev_start(). Before start \ after stop - no flow is
valid anymore.
I believe that this discussion is not about validity of flows.
Let the flows be invalid after calling to rte_eth_dev_stop().
This is OK, flows must be recreated and the bonding driver
works this way. But why not *before start*?
Think about it this way by changing the configuration you may
create invalid flows, for example, you can only change the
number of queues after port stop, so if you create a flow with
jump to queue 3 and then you remove queue 3 then, the flow that
is cached is not valid anymore. This goes for other
configuration that may affect the validity of a
flow.
This is a valid argument, of course. The thing is whether it is a
responsibility of the PMD to take care of those corner cases or if
this is up to the application developer. If we respect the fact
that calling to stop invalidates all flows then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 / end
quoted
port config
rxq 3 > port start 0
it's clear that something is really wrong with the caller/user. I
would say that this is an application bug. I would expect that you
first reconfigure port and after that you modify flows. It seems
quite logical and intuitive, doesn't it?
I agree the use case I described is bad programming, but the same
logic can be applied on stopping the port why flows should be
deleted?
I just took into account what Matan has written. Certainly, it is not necessary
to delete flows on stop.
According to documentation it is, since flows are not maintained after stop.
quoted
quoted
quoted
And even more important on some HW and some flows can only be
inserted after start since there is no real traffic gate, basically
the start means adding the flows if a flow is in the HW it is
working, adding a gate will result in PPS degradation.
What do you mean by term "gate", here? Some FPGA logic? Or something
else? Why "adding a gate" would result in PPS degradation? Can you make
those ideas a bit clearer?
Sure, I will try,
From Nvidia NIC point of view, the NIC always get traffic there is no "gate" meaning
starting the device is not just some bit that enables traffic, traffic is always there
there are number of reasons for this, (mainly the fact that Nvidia shares the device
with the kernel so any packet that the DPDK did take will be routed to the kernel)
So starting traffic means adding the flows that will direct the traffic to the queues.
Even if it was valid to add a "gate" rule it will mean that there will be an extra rule
which may lead to PPS degradation.
Is that clearer?
quoted
quoted
quoted
quoted
Anyway, the PMD can possibly catch this rxq inconsistency but I
can imagine that there are more complicated sitations than just
changing count of queues. Any idea for a more complex real case?
Some of the resource may be created only after start, But I don't
have a very good example now.
Please, try to find some reasonable example otherwise your argumentation
does not make much sense...
Agree let's leave it aside for now.
quoted
quoted
quoted
quoted
Martin Havlik, can you please check how ixgbe and i40e behave in
the case above with "rxq change after flow created"?
Both PMDs do not raise any errors in the mentioned case. The packets
that should go to the non-existent queue are dropped at some point
in the processing (from what I could see and understand).
Thanks for clarification. It would be nice if any other PMD can be tested for
this behaviour. Can sombody help us with other NICs?
quoted
quoted
I'm glad for the discussion that is taking place but I feel we have
strayed from the original reason of it. That being the inability to
enable dedicated queues for bonding mode 8023AD on mlx5. We have
concluded that flow creation has to be done after port/device start.
So moving the function call to create the flow after device start
should solve the issue
(https://mails.dpdk.org/archives/dev/2021-June/212210.html). From my
testing, flow create after start is not a problem on Intel PMDs.
It would be good to hear what net/bonding maintainers think about it.
I see no conclusion.
If net/mlx5 behaviour is fixed, will it fix the initial problem?
I believe that if mlx5 allows to create flows before calling to
rte_eth_dev_start() then the bonding would work as expected.
There are other solutions like enabling the mlx5 pmd in isolate mode.
This will require the bond device to just create the rule manually after start.
quoted
quoted
I'm turning to you, Connor, as you have made the original question
on this. Is the patch I presented applicable?
Martin
quoted
quoted
quoted
quoted
Does somebody know how other drivers behaves in this
situation? (We know and can check for Intel, there it does not
seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short) time
window between starting of a port and configuation of
filtering flows. You may want to start the port with thousands
of flows that apply just when the port starts (not after,
that's late). This may introduce glitches in filtering and
measuring of traffic (well, it is a
question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time and
even if it was the insertion is not in zero time, (assuming that
even if the flows are stored by the PMD until start they still
will not all be inserted in the same time)
Sorry, I probably do not understand well. Yes, creation of a flow
would never be zero time, agree. But if I create flows before the
port starts, than I do not have to care too much about how long
the creation takes. Because the card is expected to be configured
already before pushing the "start button".
Maybe, you assume that the created flows would first be cached
inside the PMD and when the port is finally started, it than
transparently write the queues to the HW. But this is not what I
was talking about, that's maybe even worse because you are hiding
such behaviour from users...
(I do not know the exact mlx5 implementation details, so my answer
can miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of
Nvidia we must
By "must" you probably mean "would have to"? Is that correct?
Sorry for my English but I'm not sure what is the difference,
In any case what I meant is that currently we will need to cache the flows and only
apply them after start.
quoted
quoted
quoted
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the PMD
needs to translate this difference and in this gray area there is a
game between best API best performance understanding what the user
Yes, as a former FPGA dev, I can understand your concerns. RTE Flow is a
difficult beast.
+1
Jan
quoted
quoted
quoted
really wants. This is why the doc should be very clear on what is
the expected. but this is also the reason why such document is very
hard to agree on.
Do I understand correctly that net/mlx5 maintainers will follow up?
Yes, I will follow up on this.
quoted
quoted
quoted
Ori
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?. It is
not desirable to accidently omit a packet that was received by
primary ingress logic instead of being redirected into the
dedicated queue.
Are there any chances that for mlx5 it would be possible to
insert flow rules before calling rte_eth_dev_start? Anyway,
the behaviour should be specified and documented in DPDK more
precisely to avoid such uncertainty in the future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be clarified.
It looks like there is no consensus if the patch is a right direction
or wrong. For me it looks wrong taking all above arguments in to
account (mainly necessity to be able to insert flows before pushing
start button which enables the traffic if HW supports it).
So, I'm applying first two patches and hold on this one.
From: Jan Viktorin <hidden> Date: 2021-07-14 15:00:27
On Tue, 13 Jul 2021 17:17:51 +0000
Ori Kam [off-list ref] wrote:
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Tuesday, July 13, 2021 2:06 PM
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
quoted
On 7/13/21 11:18 AM, Havlík Martin wrote:
quoted
Dne 2021-07-12 15:07, Ori Kam napsal:
quoted
Hi Jan,
quoted
-----Original Message-----
From: Jan Viktorin <redacted>
Sent: Monday, July 12, 2021 12:46 AM
On Sun, 11 Jul 2021 08:49:18 +0000 Ori Kam [off-list ref]
wrote:
quoted
Hi Jan,
Hi Ori,
quoted
quoted
-----Original Message-----
From: dev <redacted> On Behalf Of Jan Viktorin
Sent: Wednesday, July 7, 2021 6:54 PM
On Sun, 4 Jul 2021 15:18:01 +0000 Matan Azrad
[off-list ref] wrote:
quoted
From: Havlík Martin
quoted
Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
quoted
在 2021/6/22 17:25, Martin Havlik 写道:
quoted
When dedicated queues are enabled, mlx5 PMD fails
to install RTE Flows if the underlying ethdev is
not started: bond_ethdev_8023ad_flow_set(267) -
bond_ethdev_8023ad_flow_set:
quoted
quoted
port
quoted
quoted
not started (slave_port=0 queue_id=1)
Why mlx5 PMD doing flow create relys on port
started ? I noticed other PMDs did not has that
reliance.
After looking into it, I really can't answer this mlx5
centered question. Closest related info we found so
far is the 5th point in
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#cavea
ts but it only specifies it's the application's
responsibility and that flow rules are assumed invalid
after port stop/close/restart but doesn't say anything
about <stop - flow rule create - start> vs <stop -
start
- flow rule create> where the former is the point of
failure on mlx5. I'm addressing the maintainers for
mlx5, who might know a bit more on the topic.
Hello Matan,
quoted
From rte_ethdev.h
* Please note that some configuration is not stored
between
calls to
* rte_eth_dev_stop()/rte_eth_dev_start(). The following
configuration will
* be retained:
*
* - MTU
* - flow control settings
* - receive mode configuration (promiscuous mode,
all-multicast
quoted
quoted
quoted
mode,
* hardware checksum mode, RSS/VMDQ settings etc.)
* - VLAN filtering configuration
* - default MAC address
* - MAC addresses supplied to MAC address array
* - flow director filtering mode (but not filtering
rules)
* - NIC queue statistics mappings
just after this section, you can find the following
statement:
* Any other configuration will not be stored and will
need to be re-entered
* before a call to rte_eth_dev_start().
It is not very clear how is this exactly related to flows
(and this applies for all the quoted section, I think)
but at least it can
be used as a
counter argument.
quoted
quoted
I agree the doc is not clear, as I see it flows are not
part of configuration, at least not when we are talking
about rte_flow.
Agree.
quoted
quoted
quoted
Mlx5 assumes flows are allowed to be configured only
after rte_eth_dev_start(). Before start \ after stop -
no flow is valid anymore.
I believe that this discussion is not about validity of
flows. Let the flows be invalid after calling to
rte_eth_dev_stop(). This is OK, flows must be recreated
and the bonding driver works this way. But why not
*before start*?
Think about it this way by changing the configuration you
may create invalid flows, for example, you can only change
the number of queues after port stop, so if you create a
flow with jump to queue 3 and then you remove queue 3 then,
the flow that is cached is not valid anymore. This goes for
other configuration that may affect the validity of a
flow.
This is a valid argument, of course. The thing is whether it
is a responsibility of the PMD to take care of those corner
cases or if this is up to the application developer. If we
respect the fact that calling to stop invalidates all flows
then when you do:
> port stop 0
> flow create 0 ingress pattern ... / end actions queue 6 /
end
quoted
port config
rxq 3 > port start 0
it's clear that something is really wrong with the
caller/user. I would say that this is an application bug. I
would expect that you first reconfigure port and after that
you modify flows. It seems quite logical and intuitive,
doesn't it?
I agree the use case I described is bad programming, but the
same logic can be applied on stopping the port why flows
should be deleted?
I just took into account what Matan has written. Certainly, it is
not necessary to delete flows on stop.
According to documentation it is, since flows are not maintained
after stop.
So why did you ask me? :) I didn't study port-stop behaviour at all,
so it was just my general opinion. Please, leave some reference to
the documentation you are talking about.
quoted
quoted
quoted
quoted
And even more important on some HW and some flows can only be
inserted after start since there is no real traffic gate,
basically the start means adding the flows if a flow is in the
HW it is working, adding a gate will result in PPS
degradation.
What do you mean by term "gate", here? Some FPGA logic? Or something
else? Why "adding a gate" would result in PPS degradation? Can you
make those ideas a bit clearer?
Sure, I will try,
From Nvidia NIC point of view, the NIC always get traffic there is no
"gate" meaning starting the device is not just some bit that enables
traffic, traffic is always there there are number of reasons for
this, (mainly the fact that Nvidia shares the device with the kernel
so any packet that the DPDK did take will be routed to the kernel) So
starting traffic means adding the flows that will direct the traffic
to the queues. Even if it was valid to add a "gate" rule it will mean
that there will be an extra rule which may lead to PPS degradation.
Is that clearer?
Not really. You still didn't explain the term "gate".
Another think, you say "starting traffic means adding the flows that
will direct the traffic to the queues". Does the term "flows" refer to
RTE Flows? Do you mean user-inserted RTE Flows? Or are those some
internal mlx-specific configuration flows? I consider RTE Flow as a
construct created by the user of DPDK either directly or indirectly
via some DPDK library (like bonding does it). I can work with such flows
via the DPDK RTE Flow API. The "flows" you are talking about, what
do you exactly mean by that?
I did not get why there should be any extra rule. But if so, how
does it relate to PPS degradation? Where can I see such PPS degradation?
In the kernel? The DPDK application is not active yet, so I don't see
any reasons for any PPS degradation in the DPDK app. Maybe, I am
missing something...
Probably, you expect that all packets are delivered both to the kernel
and to the DPDK application (that is however not started yet) and when
DPDK app starts, it can (temporarily?) affect performace seen in the
kernel. Is it so?
Another idea I have is that inserting an "extra rule" would mean some
added latency in the processing path, but this is not PPS
degradation... What am I missing here?
How significant would such PPS degradation be?
quoted
quoted
quoted
quoted
quoted
Anyway, the PMD can possibly catch this rxq inconsistency but
I can imagine that there are more complicated sitations than
just changing count of queues. Any idea for a more complex
real case?
Some of the resource may be created only after start, But I
don't have a very good example now.
Please, try to find some reasonable example otherwise your
argumentation does not make much sense...
Agree let's leave it aside for now.
quoted
quoted
quoted
quoted
quoted
Martin Havlik, can you please check how ixgbe and i40e behave
in the case above with "rxq change after flow created"?
Both PMDs do not raise any errors in the mentioned case. The
packets that should go to the non-existent queue are dropped at
some point in the processing (from what I could see and
understand).
Thanks for clarification. It would be nice if any other PMD can be
tested for this behaviour. Can sombody help us with other NICs?
quoted
quoted
I'm glad for the discussion that is taking place but I feel we
have strayed from the original reason of it. That being the
inability to enable dedicated queues for bonding mode 8023AD on
mlx5. We have concluded that flow creation has to be done after
port/device start. So moving the function call to create the
flow after device start should solve the issue
(https://mails.dpdk.org/archives/dev/2021-June/212210.html).
From my testing, flow create after start is not a problem on
Intel PMDs.
It would be good to hear what net/bonding maintainers think about
it. I see no conclusion.
If net/mlx5 behaviour is fixed, will it fix the initial problem?
I believe that if mlx5 allows to create flows before calling to
rte_eth_dev_start() then the bonding would work as expected.
There are other solutions like enabling the mlx5 pmd in isolate mode.
This will require the bond device to just create the rule manually
after start.
That is a kind of solution, thanks for this idea. But sorry, I consider
it more as a workaround. By the way, it does not solve the issue of
inserting thousands of flows atomically upon start of the port.
quoted
quoted
quoted
I'm turning to you, Connor, as you have made the original
question on this. Is the patch I presented applicable?
Martin
quoted
quoted
quoted
quoted
Does somebody know how other drivers behaves in this
situation? (We know and can check for Intel, there it
does not seem to be an issue.)
By the way, the mlx5 behaviour opens a (probably short)
time window between starting of a port and configuation of
filtering flows. You may want to start the port with
thousands of flows that apply just when the port starts
(not after, that's late). This may introduce glitches in
filtering and measuring of traffic (well, it is a
question how
serious issue could it be...).
quoted
quoted
Agree but this is always true nothing is done is zero time
and even if it was the insertion is not in zero time,
(assuming that even if the flows are stored by the PMD
until start they still will not all be inserted in the same
time)
Sorry, I probably do not understand well. Yes, creation of a
flow would never be zero time, agree. But if I create flows
before the port starts, than I do not have to care too much
about how long the creation takes. Because the card is
expected to be configured already before pushing the "start
button".
Maybe, you assume that the created flows would first be cached
inside the PMD and when the port is finally started, it than
transparently write the queues to the HW. But this is not
what I was talking about, that's maybe even worse because you
are hiding such behaviour from users...
(I do not know the exact mlx5 implementation details, so my
answer can miss something, sorry for that.)
You are correct this is HW/PMD implementation issue, in case of
Nvidia we must
By "must" you probably mean "would have to"? Is that correct?
Sorry for my English but I'm not sure what is the difference,
Google for it, otherwise this discussion becomes really very
complicated.
In any case what I meant is that currently we will need to cache the
flows and only apply them after start.
So, does it mean that you certainly *will* work on that?
quoted
quoted
quoted
quoted
cache the flows and only insert it after start.
Since RTE flow is generic and HW is not generic sometimes the
PMD needs to translate this difference and in this gray area
there is a game between best API best performance
understanding what the user
Yes, as a former FPGA dev, I can understand your concerns. RTE Flow
is a difficult beast.
+1
quoted
Jan
quoted
quoted
quoted
really wants. This is why the doc should be very clear on what
is the expected. but this is also the reason why such document
is very hard to agree on.
Do I understand correctly that net/mlx5 maintainers will follow
up?
Yes, I will follow up on this.
quoted
quoted
quoted
quoted
Ori
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?.
It is not desirable to accidently omit a packet that was
received by primary ingress logic instead of being
redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible
to insert flow rules before calling rte_eth_dev_start?
Anyway, the behaviour should be specified and documented
in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to insert
flows before pushing start button which enables the traffic if HW
supports it).
So, I'm applying first two patches and hold on this one.
Andrew, I believe that it would be helpful to start some new thread
otherwise we would get lost here :). It seems that we will have few
more fixes for the bonding driver. Do you prefer an entirely new
patchset or v2 of this topic? Or any other advise how to proceed?
Jan
From: Thomas Monjalon <hidden> Date: 2021-07-15 13:58:58
14/07/2021 17:00, Jan Viktorin:
quoted
quoted
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
quoted
quoted
quoted
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?.
It is not desirable to accidently omit a packet that was
received by primary ingress logic instead of being
redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible
to insert flow rules before calling rte_eth_dev_start?
Anyway, the behaviour should be specified and documented
in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to insert
flows before pushing start button which enables the traffic if HW
supports it).
So, I'm applying first two patches and hold on this one.
Andrew, I believe that it would be helpful to start some new thread
otherwise we would get lost here :). It seems that we will have few
more fixes for the bonding driver. Do you prefer an entirely new
patchset or v2 of this topic? Or any other advise how to proceed?
This thread is about 3 things:
- bonding issue
- ethdev doc
- mlx5 design
That's too much topics to address in one thread :)
You may restart the discussion with a doc update
if the stop/start requirement is not clear.
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
quoted
quoted
quoted
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?.
It is not desirable to accidently omit a packet that was
received by primary ingress logic instead of being
redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible
to insert flow rules before calling rte_eth_dev_start?
Anyway, the behaviour should be specified and documented
in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to insert
flows before pushing start button which enables the traffic if HW
supports it).
So, I'm applying first two patches and hold on this one.
Andrew, I believe that it would be helpful to start some new thread
otherwise we would get lost here :). It seems that we will have few
more fixes for the bonding driver. Do you prefer an entirely new
patchset or v2 of this topic? Or any other advise how to proceed?
This thread is about 3 things:
- bonding issue
- ethdev doc
- mlx5 design
That's too much topics to address in one thread :)
You may restart the discussion with a doc update
if the stop/start requirement is not clear.
From: Jan Viktorin <hidden> Date: 2021-08-26 10:15:12
On Tue, 24 Aug 2021 14:18:16 +0100
Ferruh Yigit [off-list ref] wrote:
On 7/15/2021 2:58 PM, Thomas Monjalon wrote:
quoted
14/07/2021 17:00, Jan Viktorin:
quoted
quoted
quoted
On Tue, 13 Jul 2021 12:26:35 +0300
Andrew Rybchenko [off-list ref] wrote:
quoted
quoted
quoted
quoted
quoted
quoted
This matters for the bonding case as well, doesn't it?.
It is not desirable to accidently omit a packet that was
received by primary ingress logic instead of being
redirected into the dedicated queue.
Are there any chances that for mlx5 it would be possible
to insert flow rules before calling rte_eth_dev_start?
Anyway, the behaviour should be specified and documented
in DPDK more precisely to avoid such uncertainty in the
future.
I agree the documentation should be fixed.
+1
Cc Thomas and Ferruh since ethdev documentation should be
clarified.
It looks like there is no consensus if the patch is a right
direction or wrong. For me it looks wrong taking all above
arguments in to account (mainly necessity to be able to insert
flows before pushing start button which enables the traffic if
HW supports it).
So, I'm applying first two patches and hold on this one.
Andrew, I believe that it would be helpful to start some new thread
otherwise we would get lost here :). It seems that we will have few
more fixes for the bonding driver. Do you prefer an entirely new
patchset or v2 of this topic? Or any other advise how to proceed?
This thread is about 3 things:
- bonding issue
- ethdev doc
- mlx5 design
That's too much topics to address in one thread :)
You may restart the discussion with a doc update
if the stop/start requirement is not clear.
Is separate discussions created as follow up?
Martin is recently unavailable. But he has already started a new
thread for _ethdev doc_ topic:
[PATCH 0/4] doc: update RTE flow rule and bonding related info
https://www.mail-archive.com/dev@dpdk.org/msg214517.html
to first clarify/document what is the current status and how to proceed.
Jan