> -----Original Message-----
> From: Simon Horman <[email protected]>
> Sent: Wednesday, April 15, 2026 3:47 PM
> To: Loktionov, Aleksandr <[email protected]>
> Cc: [email protected]; Nguyen, Anthony L
> <[email protected]>; [email protected]; Avinash Dayanand
> <[email protected]>
> Subject: Re: [PATCH iwl-net 4/5] iavf: fix TC boundary check in
> iavf_handle_tclass
>
> On Mon, Apr 13, 2026 at 09:30:34AM +0200, Aleksandr Loktionov wrote:
> > From: Avinash Dayanand <[email protected]>
> >
> > The condition `tc < adapter->num_tc` admits any tc value equal to or
> > greater than num_tc, bypassing the destination-port validation and
> > allowing traffic to be steered to a non-existent traffic class.
> Change
> > the comparison to `tc > adapter->num_tc` to correctly reject
> > out-of-range TC values.
> >
> > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters")
> > Signed-off-by: Avinash Dayanand <[email protected]>
> > Signed-off-by: Aleksandr Loktionov <[email protected]>
>
> I am a bit confused by this logic.
>
> With this patch applied:
>
> 1) For tc <= adapter->num_tc, which I assume is valid TCs (other than
> 0,
> in which case the function returns earlier), the filter destination
> port
> is skipped.
>
> But the failure path for that checks logs:
> "Specify destination port to redirect to traffic class other than
> TC0\n"
>
> This does not seem consistent.
>
> 2) For tc > adapter->num_tc, which I assume is invalid TCs,
> the function will eventually assign fields of filter->f and succeed
> if filter has a valid destination port.
>
> This doesn't seem to be in keeping with the patch description.
>
> 3) The above two points aside, is there an out by 1 condition in
> the condition tc > adapter->num_tc. It seems to imply
> that tc == adapter->num_tc is a valid tc. But I suspect that
> is not hte case.
>
> In short, I'm wondering if the function should look something like
> this (completely untested):
>
> /**
> * iavf_handle_tclass - Forward to a traffic class on the device
> * @adapter: board private structure
> * @tc: traffic class index on the device
> * @filter: pointer to cloud filter structure */ static int
> iavf_handle_tclass(struct iavf_adapter *adapter, u32 tc,
> struct iavf_cloud_filter *filter) {
> if (tc == 0)
> return 0;
>
> if (tc >= adapter->num_tc) {
Yes, it looks for me now '>=' fits better here.
Thank you
> // dev_err(...);
> return -EINVAL;
> }
>
> if (!filter->f.data.tcp_spec.dst_port) {
> dev_err(&adapter->pdev->dev,
> "Specify destination port to redirect to
> traffic class other than TC0\n");
> return -EINVAL;
> }
>
> /* redirect to a traffic class on the same device */
> filter->f.action = VIRTCHNL_ACTION_TC_REDIRECT;
> filter->f.action_meta = tc;
>
> return 0;
> }
>
> > ---
> > drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > index ab5f5adc..5e4035b 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > @@ -4062,7 +4062,7 @@ static int iavf_handle_tclass(struct
> > iavf_adapter *adapter, u32 tc, {
> > if (tc == 0)
> > return 0;
> > - if (tc < adapter->num_tc) {
> > + if (tc > adapter->num_tc) {
> > if (!filter->f.data.tcp_spec.dst_port) {
> > dev_err(&adapter->pdev->dev,
> > "Specify destination port to redirect to
> traffic class other than
> > TC0\n");
> > --
> > 2.52.0
> >
