[PATCH] net/mlx5: fix connection tracking state item validation

[PATCH] net/mlx5: fix connection tracking state item validation

[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

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Ivan Malov]

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
>
>

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

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

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

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

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

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

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

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))

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

[-- 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 --]

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

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

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

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>

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

[-- 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 --]

Re: [PATCH] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

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

Hi Dariusz,
I have tested the following example and it is working fine.
Can we add this example to documentation
(either in https://doc.dpdk.org/guides/testpmd_app_ug/testpmd_funcs.html or
mlx5 or somewhere else) ?

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

>
> [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))
>

Best Regards,
Khadem

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

[PATCH v2] net/mlx5: fix connection tracking state item validation

[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

Re: [PATCH v2] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

Hi,

Thank you for the patch. Please see comments inline.

On Tue, Aug 12, 2025 at 08:46:30AM -0400, Khadem Ullah wrote:
> 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')

When used locally ./devtool/checkpatches.sh reports the following:

	WARNING:BAD_FIXES_TAG: Please use correct Fixes: style 'Fixes: <12+ chars of sha1> ("<title line>")' - ie: 'Fixes: aca19061e4b9 ("net/mlx5: validate connection tracking item")'
	#12:
	Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')

but it's not reported in "ci/checkpatch" job in Patchwork.
This check was added in Linux 6.1: https://github.com/torvalds/linux/commit/bd17e036b495bebbf07a5fc814c868e30e1dc131
so it appears that checkpatch.pl version used in CI is older than that.
Locally, I have a version from Linux 6.16.

Ali, Thomas, David: What do you think about updating the version of
checkpatch.pl used in community CI?

Khadem: Could you please fix the quotations marks in Fixes tag according to the error?

For the future, you can set up a git alias, which can be used
to automatically generate a correct tag.
It is described in: https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body

> 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);

I think it would be better to define a macro for "all possible packet state
flags", because now the definition of all flags is repeated in 2 places.
Could you please add MLX5_FLOW_CONNTRACK_PKT_STATE_ALL macro to mlx5_flow.h?

> +		if (spec->flags & ~flags_all)
> +			return rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ITEM,
> +					NULL,
> +					"Invalid CT item matching \n");

Please remove the new line character and preceding space from error message here.
rte_flow_error_set() does not print the message, it only stores
the pointer to error message in rte_flow_error struct.
Users later can print it/log it however they like.

I also propose to change the message itself, so it better pinpoints
the exact error. For example: "Invalid CT item flags".

Please apply the same to the second instance of that error.

>  	}
>  	/* 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;
> -

When you'll be sending a new version, could you reapply this empty line?
It's removal is not needed.

> /* snip rest of the diff */

Best regards,
Dariusz Sosnowski

Re: [PATCH v2] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

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

Hi Dariusz,
Thank you for your feedback.
Please check the newer version of this patch including the requested
changes.

Best regards,
Khadem

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

Re: [PATCH v2] net/mlx5: fix connection tracking state item validation

[Thomas Monjalon]

13/08/2025 16:16, Dariusz Sosnowski:
> Hi,
> 
> Thank you for the patch. Please see comments inline.
> 
> On Tue, Aug 12, 2025 at 08:46:30AM -0400, Khadem Ullah wrote:
> > 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')
> 
> When used locally ./devtool/checkpatches.sh reports the following:
> 
> 	WARNING:BAD_FIXES_TAG: Please use correct Fixes: style 'Fixes: <12+ chars of sha1> ("<title line>")' - ie: 'Fixes: aca19061e4b9 ("net/mlx5: validate connection tracking item")'
> 	#12:
> 	Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')
> 
> but it's not reported in "ci/checkpatch" job in Patchwork.
> This check was added in Linux 6.1: https://github.com/torvalds/linux/commit/bd17e036b495bebbf07a5fc814c868e30e1dc131
> so it appears that checkpatch.pl version used in CI is older than that.
> Locally, I have a version from Linux 6.16.
> 
> Ali, Thomas, David: What do you think about updating the version of
> checkpatch.pl used in community CI?

I've updated the checkpatch script used in the CI.
It's the latest version from Linux minus few non-relevant checks.

Re: [PATCH v2] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

On Mon, Aug 18, 2025 at 07:20:19PM +0200, Thomas Monjalon wrote:
> 13/08/2025 16:16, Dariusz Sosnowski:
> > Hi,
> > 
> > Thank you for the patch. Please see comments inline.
> > 
> > On Tue, Aug 12, 2025 at 08:46:30AM -0400, Khadem Ullah wrote:
> > > 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')
> > 
> > When used locally ./devtool/checkpatches.sh reports the following:
> > 
> > 	WARNING:BAD_FIXES_TAG: Please use correct Fixes: style 'Fixes: <12+ chars of sha1> ("<title line>")' - ie: 'Fixes: aca19061e4b9 ("net/mlx5: validate connection tracking item")'
> > 	#12:
> > 	Fixes: aca19061e4b9 ('net/mlx5: validate connection tracking item')
> > 
> > but it's not reported in "ci/checkpatch" job in Patchwork.
> > This check was added in Linux 6.1: https://github.com/torvalds/linux/commit/bd17e036b495bebbf07a5fc814c868e30e1dc131
> > so it appears that checkpatch.pl version used in CI is older than that.
> > Locally, I have a version from Linux 6.16.
> > 
> > Ali, Thomas, David: What do you think about updating the version of
> > checkpatch.pl used in community CI?
> 
> I've updated the checkpatch script used in the CI.
> It's the latest version from Linux minus few non-relevant checks.

Thank you Thomas

[PATCH v3] net/mlx5: fix connection tracking state item validation

[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: aca19061e4b96 ("net/mlx5: validate connection tracking item")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 drivers/net/mlx5/mlx5_flow.h    |  6 ++++++
 drivers/net/mlx5/mlx5_flow_dv.c |  8 +++++++-
 drivers/net/mlx5/mlx5_flow_hw.c | 12 ++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e890e732c3..df0f1cf064 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -100,6 +100,12 @@ enum mlx5_indirect_type {
 #define MLX5_INDIRECT_ACT_CT_GET_IDX(index) \
 	((index) & ((1 << MLX5_INDIRECT_ACT_CT_OWNER_SHIFT) - 1))
 
+#define MLX5_FLOW_CONNTRACK_PKT_STATE_ALL(flags_all) \
+	(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)
 /*
  * When HW steering flow engine is used, the CT action handles are encoded in a following way:
  * - bits 31:29 - type
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7b9e5018b8..7f68a4ef10 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,12 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  NULL,
 						  "Conflict status bits");
+		MLX5_FLOW_CONNTRACK_PKT_STATE_ALL(flags_all);
+		if (spec->flags & ~flags_all)
+			return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					NULL,
+					"Invalid CT item flags");
 	}
 	/* 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..9dd3d7325f 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -16996,7 +16996,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 +17018,16 @@ 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;
+			MLX5_FLOW_CONNTRACK_PKT_STATE_ALL(flags_all);
+			if (spec->flags & ~flags_all)
+				return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ITEM,
+						NULL,
+						"Invalid CT item flags");
+			break;
 		default:
 			break;
 		}
-- 
2.43.0

Re: [PATCH v3] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

On Thu, Aug 14, 2025 at 04:58:31AM -0400, Khadem Ullah wrote:
> [ snip ]
>
> +#define MLX5_FLOW_CONNTRACK_PKT_STATE_ALL(flags_all) \
> +	(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)

Please define this macro without setting the variable.
It should define only the constant value.

Defining this macro as proposed requires the user of that macro
to declare a variable, which is not needed for comparison against item's flags.
I mean something like this:

	if (spec->flags & ~MLX5_FLOW_CONNTRACK_PKT_STATE_ALL)

If macro is defined in the fashion allowing that,
the flags_all variable definition will not be needed.

Could you also please add an empty line between the macro definition and
the comment appearing after it, so they appear separated?

>  /*
>   * When HW steering flow engine is used, the CT action handles are encoded in a following way:
>   * - bits 31:29 - type
>
> [ snip ]

Best regards,
Dariusz Sosnowski

Re: [PATCH v3] net/mlx5: fix connection tracking state item validation

[Khadem Ullah]

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

Hi Dariusz,
Thanks for the feedback.
Please check v4 of this patch including the requested changes.

Regards,
Khadem

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

[PATCH v4] net/mlx5: fix connection tracking state item validation

[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: aca19061e4b96 ("net/mlx5: validate connection tracking item")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 drivers/net/mlx5/mlx5_flow.h    |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c |  5 +++++
 drivers/net/mlx5/mlx5_flow_hw.c | 10 ++++++++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e890e732c3..ed0c1fcfd2 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -100,6 +100,11 @@ enum mlx5_indirect_type {
 #define MLX5_INDIRECT_ACT_CT_GET_IDX(index) \
 	((index) & ((1 << MLX5_INDIRECT_ACT_CT_OWNER_SHIFT) - 1))
 
+#define MLX5_FLOW_CONNTRACK_PKT_STATE_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)
+
 /*
  * When HW steering flow engine is used, the CT action handles are encoded in a following way:
  * - bits 31:29 - type
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7b9e5018b8..f673637e7d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3289,6 +3289,11 @@ mlx5_flow_dv_validate_item_aso_ct(struct rte_eth_dev *dev,
 						  RTE_FLOW_ERROR_TYPE_ITEM,
 						  NULL,
 						  "Conflict status bits");
+		if (spec->flags & ~MLX5_FLOW_CONNTRACK_PKT_STATE_ALL)
+			return rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					NULL,
+					"Invalid CT item flags");
 	}
 	/* 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..7b279138bc 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -16996,6 +16996,7 @@ 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;
 
 		case RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT:
@@ -17016,6 +17017,15 @@ 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;
+			if (spec->flags & ~MLX5_FLOW_CONNTRACK_PKT_STATE_ALL)
+				return rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ITEM,
+						NULL,
+						"Invalid CT item flags");
+			break;
 		default:
 			break;
 		}
-- 
2.43.0

Re: [PATCH v4] net/mlx5: fix connection tracking state item validation

[Dariusz Sosnowski]

Thank you for the contribution and making the changes.

On Thu, Aug 14, 2025 at 06:16:01AM -0400, Khadem Ullah wrote:
> 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: aca19061e4b96 ("net/mlx5: validate connection tracking item")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>

Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

Best regards,
Dariusz Sosnowski

Re: [PATCH v4] net/mlx5: fix connection tracking state item validation

[Raslan Darawsheh]

Hi,

On 14/08/2025 1:16 PM, Khadem Ullah wrote:
> 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: aca19061e4b96 ("net/mlx5: validate connection tracking item")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>

Patch applied to next-net-mlx,

Kindest regards
Raslan Darawsheh

[PATCH] app/testpmd: add contrack state inspect commands commands

[Khadem Ullah]

Add command in testpmd user guide to inspect conntract CT states.

The conntract possible CT states
are SYN_RECV, ESTABLISHED, FIN_WAIT, CLOSE_WAIT, LAST_ACK and
TIME_WAIT.

Fixes: 4d07cbefe3ba0 ("app/testpmd: add commands for conntrack")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 6ad83ae50d..1653fcdbee 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -5392,6 +5392,12 @@ rules like above for the peer port.
 
  testpmd> flow indirect_action 0 update 0 action conntrack_update dir / end
 
+Inspect the conntrack action state through the following command:
+
+::
+
+ testpmd> flow indirect_action 0 query <action ID>
+
 Sample meter with policy rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0

[PATCH] app/testpmd: add contrack CT state inspect commands

[Khadem Ullah]

Add command in testpmd user guide to inspect conntract CT states.

The conntract possible CT states
are SYN_RECV, ESTABLISHED, FIN_WAIT, CLOSE_WAIT, LAST_ACK and
TIME_WAIT.

Fixes: 4d07cbefe3ba0 ("app/testpmd: add commands for conntrack")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 6ad83ae50d..1653fcdbee 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -5392,6 +5392,12 @@ rules like above for the peer port.
 
  testpmd> flow indirect_action 0 update 0 action conntrack_update dir / end
 
+Inspect the conntrack action state through the following command:
+
+::
+
+ testpmd> flow indirect_action 0 query <action ID>
+
 Sample meter with policy rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0

Re: [PATCH] app/testpmd: add contrack CT state inspect commands

[Stephen Hemminger]

On Fri, 15 Aug 2025 07:15:14 -0400
Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk> wrote:

> Add command in testpmd user guide to inspect conntract CT states.
> 
> The conntract possible CT states
> are SYN_RECV, ESTABLISHED, FIN_WAIT, CLOSE_WAIT, LAST_ACK and
> TIME_WAIT.
> 
> Fixes: 4d07cbefe3ba0 ("app/testpmd: add commands for conntrack")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---

Queued to next-net after fixing spelling in commit message.

Re: [PATCH] app/testpmd: add contrack CT state inspect commands

[Khadem Ullah]

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

Hi Stephen,
Thanks for the update.
Should I correct the spelling in the commit message in the next version of
the patch ?


On Wed, Aug 20, 2025 at 11:48 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

>
> Queued to next-net after fixing spelling in commit message.
>


Best Regards,
Khadem

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

[PATCH v2] net/mlx5: fix connection tracking state item validation

[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