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

[Dariusz Sosnowski <dsosnowski@nvidia.com>]

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