DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/mlx5: fix connection tracking state item validation
@ 2025-08-05 13:23 Khadem Ullah
  2025-08-05 14:44 ` Ivan Malov
  2025-08-12 12:46 ` [PATCH v2] " Khadem Ullah
  0 siblings, 2 replies; 11+ messages in thread
From: Khadem Ullah @ 2025-08-05 13:23 UTC (permalink / raw)
  To: Dariusz Sosnowski, Viacheslav Ovsiienko, Bing Zhao, Ori Kam,
	Suanming Mou, Matan Azrad
  Cc: dev, stable, Khadem Ullah

This patch validate a connection tracking state when matching
'conntrack is' in rte_flow rules. The conntract possible CT states
are SYN_RECV, ESTABLISHED, FIN_WAIT, CLOSE_WAIT, LAST_ACK and
TIME_WAIT. Therefore the maximum possible value to match on
in rte_flow is TIME_WAIT but mlx5 allowed matching on any values.

This patch validate the CT state item.
Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7b9e5018b8..750385cd42 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3290,6 +3290,11 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
 						  NULL,
 						  "Conflict status bits");
 	}
+	if (spec->flags > RTE_FLOW_CONNTRACK_STATE_TIME_WAIT)
+		return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					NULL,
+					"Invalid CT state matching \n");
 	/* State change also needs to be considered. */
 	*item_flags |= MLX5_FLOW_LAYER_ASO_CT;
 	return 0;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-05 13:23 [PATCH] net/mlx5: fix connection tracking state item validation Khadem Ullah
@ 2025-08-05 14:44 ` Ivan Malov
  2025-08-06  8:51   ` Khadem Ullah
  2025-08-12 12:46 ` [PATCH v2] " Khadem Ullah
  1 sibling, 1 reply; 11+ messages in thread
From: Ivan Malov @ 2025-08-05 14:44 UTC (permalink / raw)
  To: Khadem Ullah
  Cc: Dariusz Sosnowski, Viacheslav Ovsiienko, Bing Zhao, Ori Kam,
	Suanming Mou, Matan Azrad, dev, stable

Hi,

On Tue, 5 Aug 2025, Khadem Ullah wrote:

> This patch validate a connection tracking state when matching
> 'conntrack is' in rte_flow rules. The conntract possible CT states
> are SYN_RECV, ESTABLISHED, FIN_WAIT, CLOSE_WAIT, LAST_ACK and
> TIME_WAIT. Therefore the maximum possible value to match on
> in rte_flow is TIME_WAIT but mlx5 allowed matching on any values.
>
> This patch validate the CT state item.
> Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')
> Cc: stable@dpdk.org
>
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 7b9e5018b8..750385cd42 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3290,6 +3290,11 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
> 						  NULL,
> 						  "Conflict status bits");
> 	}
> +	if (spec->flags > RTE_FLOW_CONNTRACK_STATE_TIME_WAIT)
> +		return rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ITEM,
> +					NULL,
> +					"Invalid CT state matching \n");

It might be better to enclose the multi-line block in brackets.

Also, is it correct to treat 'flags' like enum 'RTE_FLOW_CONNTRACK_STATE'?
I thought it was following 'RTE_FLOW_CONNTRACK_PKT_STATE' flags instead [1].

[1] https://doc.dpdk.org/api-25.07/rte__flow_8h.html#a7a41946aa03ebca8c432279604265b51

Or am I missing something?

Thank you.

> 	/* State change also needs to be considered. */
> 	*item_flags |= MLX5_FLOW_LAYER_ASO_CT;
> 	return 0;
> -- 
> 2.43.0
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-05 14:44 ` Ivan Malov
@ 2025-08-06  8:51   ` Khadem Ullah
  2025-08-08  7:47     ` Dariusz Sosnowski
  0 siblings, 1 reply; 11+ messages in thread
From: Khadem Ullah @ 2025-08-06  8:51 UTC (permalink / raw)
  To: ivan.malov
  Cc: dsosnowski, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

Hi Ivan, 

The multi-line block was not closed in brackets due to coding style.

Spec provides values to match (e.g. a given IPv4 address) in rte_flow rules. 
Incase of the flowing rte_flow rule, spec provides in 
mlx5_flow_dv_validate_item_aso_ct the conntrack state to be validated. 

The following is a valid rule and it should be offloaded. 

flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 5 / end

and the following is an invalid rule which should be rejected, as there is no conntract state corresponds to 10.
flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 10 / end actions queue index 5 / end

Please check https://doc.dpdk.org/guides-24.07/prog_guide/rte_flow.html#pattern-item

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-06  8:51   ` Khadem Ullah
@ 2025-08-08  7:47     ` Dariusz Sosnowski
  2025-08-11  6:21       ` Khadem Ullah
  0 siblings, 1 reply; 11+ messages in thread
From: Dariusz Sosnowski @ 2025-08-08  7:47 UTC (permalink / raw)
  To: Khadem Ullah
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

On Wed, Aug 06, 2025 at 04:51:42AM -0400, Khadem Ullah wrote:
> Hi Ivan, 
> 
> The multi-line block was not closed in brackets due to coding style.
> 
> Spec provides values to match (e.g. a given IPv4 address) in rte_flow rules. 
> Incase of the flowing rte_flow rule, spec provides in 
> mlx5_flow_dv_validate_item_aso_ct the conntrack state to be validated. 
> 
> The following is a valid rule and it should be offloaded. 
> 
> flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 5 / end
> 
> and the following is an invalid rule which should be rejected, as there is no conntract state corresponds to 10.
> flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 10 / end actions queue index 5 / end
> 
> Please check https://doc.dpdk.org/guides-24.07/prog_guide/rte_flow.html#pattern-item

I think there might have been a misunderstanding about states
relevant for conntrack flow items and actions.
What Ivan has mentioned regarding RTE_FLOW_CONNTRACK_PKT_STATE_* flags
is correct.

- rte_flow_conntrack_state enum denotes possible TCP connection states
  (SYN_RECV, ESTABLISHED, TIME_WAIT, etc.).
  These are used during creation/querying of the conntrack flow action object
  to initialize/inspect the TCP connection state machine in the HW.
  From flow API perspective, it is relevant only for the conntrack action:
  https://doc.dpdk.org/api/structrte__flow__action__conntrack.html

- rte_flow_item_conntrack flow item is used to match packets based on
  how they interact with HW TCP state machine.
  After conntrack action is executed in the HW on the packet,
  application can match that packet based on the result of connection
  tracking e.g., match correct TCP packets which are in current TCP
  window or match packets which change TCP connection state.

  As noted in API docs - https://doc.dpdk.org/api/structrte__flow__item__conntrack.html -
  this item takes a bitmap of RTE_FLOW_CONNTRACK_PKT_STATE_* bits.
  For example match can be on RTE_FLOW_CONNTRACK_PKT_STATE_VALID | RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED
  meaning, "match valid TCP packets which change TCP connection state".

Because of the above, the proposed change to validate item's flags
against variants of rte_flow_conntrack_state enum is incorrect.

Best regards,
Dariusz Sosnowski

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-08  7:47     ` Dariusz Sosnowski
@ 2025-08-11  6:21       ` Khadem Ullah
  2025-08-11 15:15         ` Dariusz Sosnowski
  0 siblings, 1 reply; 11+ messages in thread
From: Khadem Ullah @ 2025-08-11  6:21 UTC (permalink / raw)
  To: dsosnowski, ivan.malov
  Cc: viacheslavo, bingz, orika, suanmingm, matan, dev, stable

Hi Dariusz Sosnowski, 

According to documentation, conntrack item matches a conntrack 
state after conntrack action. Your statement is also correct 
"match valid TCP packets which change TCP connection state", 
it means in this case also we are matching TCP connection state. 

Please check CONNTRACK item in Generic flow API (rte_flow)
16.2.6.47. Item: CONNTRACK

Matches a conntrack state after conntrack action.

    flags: conntrack packet state flags.
    Default mask matches all state bits.

https://doc.dpdk.org/guides-24.07/prog_guide/rte_flow.html

E.g. I have performed the following experiemtns on connect-x6-Dx for clarification
(provding only the relevent information).

conntract state can be verified for liberal mode.
In liberal mode, the Seq/ACK/Win check will be ignored (forget about act/seq)
and only the state change will be tracked.

Test 1 : Starting state machine from State 0  

flow indirect_action 0 create ingress action conntrack / end
flow create 0 ingress pattern eth / ipv4 / tcp / end actions jump group 3 / end
flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end actions indirect 0 / jump group 5 / end
flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 1 / end
flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 2 / end actions queue index 2 / end
set fwd rxonly 
start 
set verbose 3 

The following packets will be forwared to queue 1, it means the state machine is now in established state (state 1). 
sendp(Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10), iface="",count=1)  
sendp(Ether(dst="bb:cc:dd:ee:ff:22",src="aa:bb:cc:dd:ee:ff")/IP(src="150.1.10.10")/TCP(ack=265,seq=265,dport=5555,flags=0x18), iface="",count=1)

FIN packet Terminate the connection; the following packets will be forwarded to queue 2:
pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x01)
pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)
pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x01)
pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)

This will be again forwarded it to queue 1: 
pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)

So, according to my understanding(from rte_flow and various experiments), 
conntrack item ('conntract is') matches the state of the connection tracking 
state machine in hardware 
and forward it to the relevent queue. 

In any case, I think only a range of values for "conntract is" to be allowed. 

Best Regards, 
Khadem

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-11  6:21       ` Khadem Ullah
@ 2025-08-11 15:15         ` Dariusz Sosnowski
  2025-08-11 16:27           ` Khadem Ullah
  0 siblings, 1 reply; 11+ messages in thread
From: Dariusz Sosnowski @ 2025-08-11 15:15 UTC (permalink / raw)
  To: Khadem Ullah
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

On Mon, Aug 11, 2025 at 02:21:49AM -0400, Khadem Ullah wrote:
> Hi Dariusz Sosnowski, 
> 
> According to documentation, conntrack item matches a conntrack 
> state after conntrack action. Your statement is also correct 
> "match valid TCP packets which change TCP connection state", 
> it means in this case also we are matching TCP connection state. 
> 
> Please check CONNTRACK item in Generic flow API (rte_flow)
> 16.2.6.47. Item: CONNTRACK
> 
> Matches a conntrack state after conntrack action.
> 
>     flags: conntrack packet state flags.
>     Default mask matches all state bits.
> 
> https://doc.dpdk.org/guides-24.07/prog_guide/rte_flow.html

As mentioned in the quoted docs - flags in conntrack item match
"conntrack packet state flags", not "connection state".
Packet state refers to one or more RTE_FLOW_CONNTRACK_PKT_STATE_* flags.
(Documented specifically in API docs).

I am focusing on highlighting the difference between "packet state"
and "connection state", since this distinction is important, because:

- flow item deals with "packet state"
- flow action deals with "connection state"

One of the flags for "packet state" is RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED,
which indicates that this packet has changed TCP connection state.
But without analyzing the packet and inspecting TCP connection state
(by querying the conntrack flow action),
user does not know the specific state transition i.e.,
whether transition was SYN_RECV -> ESTABLISHED, or ESTABLISHED -> FIN_WAIT for example.
So user can match packets which changed the connection state,
but not the specific state transition.

Perhaps the docs (both programming guide and API docs) could be improved
in that regard to highlight the difference more clearly.

> 
> E.g. I have performed the following experiemtns on connect-x6-Dx for clarification
> (provding only the relevent information).
> 
> conntract state can be verified for liberal mode.
> In liberal mode, the Seq/ACK/Win check will be ignored (forget about act/seq)
> and only the state change will be tracked.

Correct, but even with liberal mode (so no TCP window check)
"packet state" after conntrack execution is still at least
RTE_FLOW_CONNTRACK_PKT_STATE_VALID.
This is the area of the docs which should be improved, since it is not
stated.

This makes the following example matches correct
(take note that flow item takes a bitmap of RTE_FLOW_CONNTRACK_PKT_STATE_*):

- conntrack is 1 => RTE_FLOW_CONNTRACK_PKT_STATE_VALID

- conntrack is 2 => RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED

- conntrack is 3 => RTE_FLOW_CONNTRACK_PKT_STATE_VALID **and**
                    RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED

> 
> Test 1 : Starting state machine from State 0  
> 
> flow indirect_action 0 create ingress action conntrack / end
> flow create 0 ingress pattern eth / ipv4 / tcp / end actions jump group 3 / end
> flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end actions indirect 0 / jump group 5 / end
> flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 1 / end
> flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 2 / end actions queue index 2 / end
> set fwd rxonly 
> start 
> set verbose 3 
> 
> The following packets will be forwared to queue 1, it means the state machine is now in established state (state 1). 
> sendp(Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10), iface="",count=1)  
> sendp(Ether(dst="bb:cc:dd:ee:ff:22",src="aa:bb:cc:dd:ee:ff")/IP(src="150.1.10.10")/TCP(ack=265,seq=265,dport=5555,flags=0x18), iface="",count=1)
> 
> FIN packet Terminate the connection; the following packets will be forwarded to queue 2:
> pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x01)
> pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)
> pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x01)
> pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)
> 
> This will be again forwarded it to queue 1: 
> pkt=Ether()/IP()/TCP(ack=265,seq=265,dport=5555,flags=0x10)

I'm not sure how this result was reached, because when using these commands
and sending these packets, I do not receive any.
I only receive the packets if and only if the rule with conntrack item
matches on RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED:

	flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 8 / end actions queue index 0 / end

Which is expected with the given configuration.

Are these the only testpmd commands you execute?

If yes, then there are a few things missing for functional conntrack offload.
Let me explain:

1. There is no conntrack action configuration.

   testpmd has a few commands which allow users to provide initial
   conntrack action state. Please see https://doc.dpdk.org/guides/testpmd_app_ug/testpmd_funcs.html#sample-conntrack-rules

   Without running these commands, initial conntrack state is zeroed.
   As a result, `enable` field in `rte_flow_action_conntrack` is zero
   and TCP state machine in HW is not updated for each passing packet,
   and each packet is marked with RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED.

2. There is only one rule with conntrack action.

   Rules with conntrack action need to know which TCP connection direction
   would pass through that rule. This is needed because, conntrack
   offload does not track IP addresses and TCP ports.
   Inside the state machine, there are 2 separate sets of seq/ack
   numbers tracked for each direction.

   Users must ensure that there will be 2 rules (1 for each TCP
   direction) with conntrack actions.

3. conntrack item deals with RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap

   In your example, "conntrack is 1" specification sets flags to 1.
   This means, "match packets with RTE_FLOW_CONNTRACK_PKT_STATE_VALID"
   and not "connection in RTE_FLOW_CONNTRACK_STATE_ESTABLISHED".

   The same goes for "conntrack is 2". It specifies match on
   RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED, not on
   RTE_FLOW_CONNTRACK_STATE_FIN_WAIT or any other state.

   Because it is a bitmap, conntrack item can specify a combination of
   PKT_STATE flags. For example, "conntrack is 3" would mean matching
   a packet with RTE_FLOW_CONNTRACK_PKT_STATE_VALID and
   RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED flags set.

Please see [1] for a full example. Let us know if you have any questions
about the example and if it works for you.

> 
> So, according to my understanding(from rte_flow and various experiments), 
> conntrack item ('conntract is') matches the state of the connection tracking 
> state machine in hardware 
> and forward it to the relevent queue. 
> 
> In any case, I think only a range of values for "conntract is" to be allowed. 
> 
> Best Regards, 
> Khadem

[1]: Full conntrack example, testpmd commands:

# Initial conntrack action configuration: original direction, state SYN_RECV, liberal mode and enabled
set conntrack com peer 0 is_orig 1 enable 1 live 0 sack 0 cack 0 last_dir 0 liberal 1 state 0 max_ack_win 0 r_lim 5 last_win 510 last_seq 2000 last_ack 101 last_end 101 last_index 0x2
set conntrack orig scale 0xf fin 0 acked 1 unack_data 0 sent_end 101 reply_end 65535 max_win 0 max_ack 0
set conntrack rply scale 0xf fin 0 acked 1 unack_data 0 sent_end 2001 reply_end 65535 max_win 0 max_ack 101
flow indirect_action 0 create ingress action conntrack / end

# Create a rule for original direction
flow create 0 group 3 ingress pattern eth / ipv4 src is 1.2.3.4 dst is 5.6.7.8 / tcp src is 40000 dst is 50000 / end actions indirect 0 / jump group 5 / end

# Update conntrack action - now rule will created for reply direction
set conntrack com peer 0 is_orig 0 enable 1 live 0 sack 0 cack 0 last_dir 0 liberal 1 state 0 max_ack_win 0 r_lim 5 last_win 510 last_seq 2000 last_ack 101 last_end 101 last_index 0x2
flow indirect_action 0 update 0 action conntrack_update dir / end

# Create a rule for reply direction
flow create 0 group 3 ingress pattern eth / ipv4 src is 5.6.7.8 dst is 1.2.3.4 / tcp src is 50000 dst is 40000 / end actions indirect 0 / jump group 5 / end

# Create group 0 rule for TCP traffic
flow create 0 ingress pattern eth / ipv4 / tcp / end actions jump group 3 / end

# Match valid packets, mark and send to queue 0
flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions mark id 0x111 / queue index 0 / end
# Match valid packets which change connection state
flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 3 / end actions mark id 0x333 / queue index 0 / end

set verbose 1
set fwd rxonly
start

Example packets to send after all flow rules are created:

# ACK in handshake: transition SYN_RECV->ESTABLISHED; logged as "FDIR matched ID=0x333"
pkt = (Ether() / IP(src='1.2.3.4', dst='5.6.7.8') / TCP(sport=40000, dport=50000, flags='A', seq=101, ack=2001))

# some data from original direction; logged as "FDIR matched ID=0x111"
pkt = (Ether() / IP(src='1.2.3.4', dst='5.6.7.8') / TCP(sport=40000, dport=50000, flags='A', seq=101, ack=2001) / Raw(load=b'a' * 100))

# ack from reply direction; logged as "FDIR matched ID=0x111"
pkt = (Ether() / IP(src='5.6.7.8', dst='1.2.3.4') / TCP(sport=50000, dport=40000, flags='A', seq=2001, ack=201))

# fin from original direction; logged as "FDIR matched ID=0x333"
pkt = (Ether() / IP(src='1.2.3.4', dst='5.6.7.8') / TCP(sport=40000, dport=50000, flags='F', seq=201, ack=2001))

# ack from reply direction; logged as "FDIR matched ID=0x333"
pkt = (Ether() / IP(src='5.6.7.8', dst='1.2.3.4') / TCP(sport=50000, dport=40000, flags='A', seq=2001, ack=202))

# fin from reply direction; logged as "FDIR matched ID=0x333"
pkt = (Ether() / IP(src='5.6.7.8', dst='1.2.3.4') / TCP(sport=50000, dport=40000, flags='F', seq=2001, ack=202))

# ack from original direction; logged as "FDIR matched ID=0x333"
pkt = (Ether() / IP(src='1.2.3.4', dst='5.6.7.8') / TCP(sport=40000, dport=50000, flags='A', seq=201, ack=2002))

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-11 15:15         ` Dariusz Sosnowski
@ 2025-08-11 16:27           ` Khadem Ullah
  2025-08-11 17:18             ` Dariusz Sosnowski
  0 siblings, 1 reply; 11+ messages in thread
From: Khadem Ullah @ 2025-08-11 16:27 UTC (permalink / raw)
  To: Dariusz Sosnowski
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

Thank you for providing these details. Sure, I will go through it (will
performed the experiment) and come back to you.
I totally agree that the documentation about connection tracking should be
improved.


On Mon, Aug 11, 2025 at 8:17 PM Dariusz Sosnowski <dsosnowski@nvidia.com>
wrote:

>
> > Are these the only testpmd commands you execute?
>
> No, as I mentioned earlier, I have provided only relevant information. I
had added something similar commands as yours,
the following was missing from my configurations.

set conntrack com peer 1 is_orig 1 enable 1 live 1 sack 1 cack 0 last_dir 0
liberal 1 state 0 max_ack_win 7
r_lim 3 last_win 510 last_seq 65535 last_ack 65537 last_end 65545
last_index 0x8

set conntrack orig scale 7 fin 1 acked 1 unack_data 0 sent_end 65545
reply_end 65535 max_win 28960 max_ack 2632987379
set conntrack rply scale 7 fin 0 acked 1 unack_data 0 sent_end 65545
reply_end 65535 max_win 65280 max_ack 2532480967

. > 3 conntrack item deals with RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap

  > In your example, "conntrack is 1" specification sets flags to 1.
   > This means, "match packets with RTE_FLOW_CONNTRACK_PKT_STATE_VALID"
   >and not "connection in RTE_FLOW_CONNTRACK_STATE_ESTABLISHED".

   > The same goes for "conntrack is 2". It specifies match on
   > RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED, not on
   >R TE_FLOW_CONNTRACK_STATE_FIN_WAIT or any other state.
>
  > Because it is a bitmap, conntrack item can specify a combination of
   >P KT_STATE flags. For example, "conntrack is 3" would mean matching
   >a packet with RTE_FLOW_CONNTRACK_PKT_STATE_VALID and
   >RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED flags set.

Can this RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap be represented with a
specific valid range ?
for example, we can say, 'conntrack is' valid for 1 to 8, or any other
range. As, currently user can specify
any value e.g., 1000 and it allows it.

Thanks again!
Best regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 2633 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-11 16:27           ` Khadem Ullah
@ 2025-08-11 17:18             ` Dariusz Sosnowski
  2025-08-12  9:51               ` Dariusz Sosnowski
  0 siblings, 1 reply; 11+ messages in thread
From: Dariusz Sosnowski @ 2025-08-11 17:18 UTC (permalink / raw)
  To: Khadem Ullah
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

On Mon, Aug 11, 2025 at 09:27:06PM +0500, Khadem Ullah wrote:
> Thank you for providing these details. Sure, I will go through it (will
> performed the experiment) and come back to you.
> I totally agree that the documentation about connection tracking should be
> improved.
> 
> 
> On Mon, Aug 11, 2025 at 8:17 PM Dariusz Sosnowski <dsosnowski@nvidia.com>
> wrote:
> 
> >
> > > Are these the only testpmd commands you execute?
> >
> > No, as I mentioned earlier, I have provided only relevant information. I
> had added something similar commands as yours,
> the following was missing from my configurations.
> 
> set conntrack com peer 1 is_orig 1 enable 1 live 1 sack 1 cack 0 last_dir 0
> liberal 1 state 0 max_ack_win 7
> r_lim 3 last_win 510 last_seq 65535 last_ack 65537 last_end 65545
> last_index 0x8
> 
> set conntrack orig scale 7 fin 1 acked 1 unack_data 0 sent_end 65545
> reply_end 65535 max_win 28960 max_ack 2632987379
> set conntrack rply scale 7 fin 0 acked 1 unack_data 0 sent_end 65545
> reply_end 65535 max_win 65280 max_ack 2532480967
> 
> . > 3 conntrack item deals with RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap
> 
>   > In your example, "conntrack is 1" specification sets flags to 1.
>    > This means, "match packets with RTE_FLOW_CONNTRACK_PKT_STATE_VALID"
>    >and not "connection in RTE_FLOW_CONNTRACK_STATE_ESTABLISHED".
> 
>    > The same goes for "conntrack is 2". It specifies match on
>    > RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED, not on
>    >R TE_FLOW_CONNTRACK_STATE_FIN_WAIT or any other state.
> >
>   > Because it is a bitmap, conntrack item can specify a combination of
>    >P KT_STATE flags. For example, "conntrack is 3" would mean matching
>    >a packet with RTE_FLOW_CONNTRACK_PKT_STATE_VALID and
>    >RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED flags set.
> 
> Can this RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap be represented with a
> specific valid range ?
> for example, we can say, 'conntrack is' valid for 1 to 8, or any other
> range. As, currently user can specify
> any value e.g., 1000 and it allows it.

Since conntrack item flags is a bitmap, then any combination of RTE_FLOW_CONNTRACK_PKT_STATE_*
flags is a valid value to match on.

The validation could be done as follows:

	flags_all = (RTE_FLOW_CONNTRACK_PKT_STATE_VALID |
	             RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED |
	             RTE_FLOW_CONNTRACK_PKT_STATE_INVALID |
	             RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED |
	             RTE_FLOW_CONNTRACK_PKT_STATE_BAD)

	if spec->flags & ~flags_all:
	    reject

Regarding validation itself, if this is added, please make sure of the following:

- In mlx5_flow_dv_validate_item_aso_ct() - check for spec->flags should be
  inside if clause for `!mlx5_hws_active()`. This is to make sure that
  validation is done only in synchronous flow API.
- Asynchronous flow API has a separate validation, which can be enabled
  at build time.
  This can be added to a switch case in flow_hw_validate_rule_pattern().

> 
> Thanks again!
> Best regards,
> Khadem

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-11 17:18             ` Dariusz Sosnowski
@ 2025-08-12  9:51               ` Dariusz Sosnowski
  2025-08-12 12:50                 ` Khadem Ullah
  0 siblings, 1 reply; 11+ messages in thread
From: Dariusz Sosnowski @ 2025-08-12  9:51 UTC (permalink / raw)
  To: Khadem Ullah
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

On Mon, Aug 11, 2025 at 07:18:34PM +0200, Dariusz Sosnowski wrote:
> On Mon, Aug 11, 2025 at 09:27:06PM +0500, Khadem Ullah wrote:
> > Thank you for providing these details. Sure, I will go through it (will
> > performed the experiment) and come back to you.
> > I totally agree that the documentation about connection tracking should be
> > improved.
> > 
> > 
> > On Mon, Aug 11, 2025 at 8:17 PM Dariusz Sosnowski <dsosnowski@nvidia.com>
> > wrote:
> > 
> > >
> > > > Are these the only testpmd commands you execute?
> > >
> > > No, as I mentioned earlier, I have provided only relevant information. I
> > had added something similar commands as yours,
> > the following was missing from my configurations.
> > 
> > set conntrack com peer 1 is_orig 1 enable 1 live 1 sack 1 cack 0 last_dir 0
> > liberal 1 state 0 max_ack_win 7
> > r_lim 3 last_win 510 last_seq 65535 last_ack 65537 last_end 65545
> > last_index 0x8
> > 
> > set conntrack orig scale 7 fin 1 acked 1 unack_data 0 sent_end 65545
> > reply_end 65535 max_win 28960 max_ack 2632987379
> > set conntrack rply scale 7 fin 0 acked 1 unack_data 0 sent_end 65545
> > reply_end 65535 max_win 65280 max_ack 2532480967
> > 
> > . > 3 conntrack item deals with RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap
> > 
> >   > In your example, "conntrack is 1" specification sets flags to 1.
> >    > This means, "match packets with RTE_FLOW_CONNTRACK_PKT_STATE_VALID"
> >    >and not "connection in RTE_FLOW_CONNTRACK_STATE_ESTABLISHED".
> > 
> >    > The same goes for "conntrack is 2". It specifies match on
> >    > RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED, not on
> >    >R TE_FLOW_CONNTRACK_STATE_FIN_WAIT or any other state.
> > >
> >   > Because it is a bitmap, conntrack item can specify a combination of
> >    >P KT_STATE flags. For example, "conntrack is 3" would mean matching
> >    >a packet with RTE_FLOW_CONNTRACK_PKT_STATE_VALID and
> >    >RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED flags set.
> > 
> > Can this RTE_FLOW_CONNTRACK_PKT_STATE_* bitmap be represented with a
> > specific valid range ?
> > for example, we can say, 'conntrack is' valid for 1 to 8, or any other
> > range. As, currently user can specify
> > any value e.g., 1000 and it allows it.
> 
> Since conntrack item flags is a bitmap, then any combination of RTE_FLOW_CONNTRACK_PKT_STATE_*
> flags is a valid value to match on.
> 
> The validation could be done as follows:
> 
> 	flags_all = (RTE_FLOW_CONNTRACK_PKT_STATE_VALID |
> 	             RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED |
> 	             RTE_FLOW_CONNTRACK_PKT_STATE_INVALID |
> 	             RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED |
> 	             RTE_FLOW_CONNTRACK_PKT_STATE_BAD)
> 
> 	if spec->flags & ~flags_all:
> 	    reject
> 
> Regarding validation itself, if this is added, please make sure of the following:
> 
> - In mlx5_flow_dv_validate_item_aso_ct() - check for spec->flags should be
>   inside if clause for `!mlx5_hws_active()`. This is to make sure that
>   validation is done only in synchronous flow API.
> - Asynchronous flow API has a separate validation, which can be enabled
>   at build time.
>   This can be added to a switch case in flow_hw_validate_rule_pattern().
> 

Khadem, please note this patch: https://patches.dpdk.org/project/dpdk/patch/20250812094323.712559-1-dsosnowski@nvidia.com/

This would allow you to inspect the conntrack action state
through the following testpmd command:

	flow indirect_action 0 query <action ID>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] net/mlx5: fix connection tracking state item validation
  2025-08-05 13:23 [PATCH] net/mlx5: fix connection tracking state item validation Khadem Ullah
  2025-08-05 14:44 ` Ivan Malov
@ 2025-08-12 12:46 ` Khadem Ullah
  1 sibling, 0 replies; 11+ messages in thread
From: Khadem Ullah @ 2025-08-12 12:46 UTC (permalink / raw)
  To: Dariusz Sosnowski, Viacheslav Ovsiienko, Bing Zhao, Ori Kam,
	Suanming Mou, Matan Azrad
  Cc: dev, stable, Khadem Ullah

This patch validate a connection tracking state when matching
'conntrack is' in rte_flow rules. Since conntrack item flags
is a bitmap, then any combination of RTE_FLOW_CONNTRACK_PKT_STATE_*
flags is a valid value to match on.

This patch validate the CT state item.
Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 12 +++++++++++-
 drivers/net/mlx5/mlx5_flow_hw.c | 17 ++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7b9e5018b8..19475b931f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3271,7 +3271,7 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
 {
 	const struct rte_flow_item_conntrack *spec = item->spec;
 	const struct rte_flow_item_conntrack *mask = item->mask;
-	uint32_t flags;
+	uint32_t flags, flags_all;
 
 	if (*item_flags & MLX5_FLOW_LAYER_ASO_CT)
 		return rte_flow_error_set(error, EINVAL,
@@ -3289,6 +3289,16 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  NULL,
 						  "Conflict status bits");
+		flags_all = (RTE_FLOW_CONNTRACK_PKT_STATE_VALID |
+				RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED |
+				RTE_FLOW_CONNTRACK_PKT_STATE_INVALID |
+				RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED |
+				RTE_FLOW_CONNTRACK_PKT_STATE_BAD);
+		if (spec->flags & ~flags_all)
+			return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					NULL,
+					"Invalid CT item matching \n");
 	}
 	/* State change also needs to be considered. */
 	*item_flags |= MLX5_FLOW_LAYER_ASO_CT;
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 6dc16f80d3..6dbbc44819 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -16957,7 +16957,6 @@ flow_hw_validate_rule_pattern(struct rte_eth_dev *dev,
 {
 	const struct rte_flow_pattern_template *pt;
 	const struct rte_flow_item *pt_item;
-
 	if (pattern_template_idx >= table->nb_item_templates)
 		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "Pattern template index out of range");
@@ -16996,7 +16995,9 @@ flow_hw_validate_rule_pattern(struct rte_eth_dev *dev,
 		switch (items->type) {
 		const struct rte_flow_item_ethdev *ethdev;
 		const struct rte_flow_item_tx_queue *tx_queue;
+		const struct rte_flow_item_conntrack *spec;
 		struct mlx5_txq_ctrl *txq;
+		uint32_t flags_all;
 
 		case RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT:
 			ethdev = items->spec;
@@ -17016,6 +17017,20 @@ flow_hw_validate_rule_pattern(struct rte_eth_dev *dev,
 							  RTE_FLOW_ERROR_TYPE_ITEM_SPEC, items,
 							  "Invalid Tx queue");
 			mlx5_txq_release(dev, tx_queue->tx_queue);
+			break;
+		case RTE_FLOW_ITEM_TYPE_CONNTRACK:
+			spec = items->spec;
+			flags_all = (RTE_FLOW_CONNTRACK_PKT_STATE_VALID |
+					RTE_FLOW_CONNTRACK_PKT_STATE_CHANGED |
+					RTE_FLOW_CONNTRACK_PKT_STATE_INVALID |
+					RTE_FLOW_CONNTRACK_PKT_STATE_DISABLED |
+					RTE_FLOW_CONNTRACK_PKT_STATE_BAD);
+			if (spec->flags & ~flags_all)
+				return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ITEM,
+						NULL,
+						"Invalid CT item matching \n");
+			break;
 		default:
 			break;
 		}
-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] net/mlx5: fix connection tracking state item validation
  2025-08-12  9:51               ` Dariusz Sosnowski
@ 2025-08-12 12:50                 ` Khadem Ullah
  0 siblings, 0 replies; 11+ messages in thread
From: Khadem Ullah @ 2025-08-12 12:50 UTC (permalink / raw)
  To: Dariusz Sosnowski
  Cc: ivan.malov, viacheslavo, bingz, orika, suanmingm, matan, dev, stable

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

Hi Dariusz,
Thank you for the update. I have added a newer version of this patch,
please check it.
https://patches.dpdk.org/project/dpdk/patch/20250812124630.2916225-1-14pwcse1224@uetpeshawar.edu.pk/

On Tue, Aug 12, 2025 at 2:53 PM Dariusz Sosnowski <dsosnowski@nvidia.com>
wrote:

>
> Khadem, please note this patch:
> https://patches.dpdk.org/project/dpdk/patch/20250812094323.712559-1-dsosnowski@nvidia.com/
>
> This would allow you to inspect the conntrack action state
> through the following testpmd command:
>
>         flow indirect_action 0 query <action ID>
>

Thanks & Best Regards,
Khadem

[-- Attachment #2: Type: text/html, Size: 1256 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-12 12:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-05 13:23 [PATCH] net/mlx5: fix connection tracking state item validation Khadem Ullah
2025-08-05 14:44 ` Ivan Malov
2025-08-06  8:51   ` Khadem Ullah
2025-08-08  7:47     ` Dariusz Sosnowski
2025-08-11  6:21       ` Khadem Ullah
2025-08-11 15:15         ` Dariusz Sosnowski
2025-08-11 16:27           ` Khadem Ullah
2025-08-11 17:18             ` Dariusz Sosnowski
2025-08-12  9:51               ` Dariusz Sosnowski
2025-08-12 12:50                 ` Khadem Ullah
2025-08-12 12:46 ` [PATCH v2] " Khadem Ullah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).