Re: [PATCH net-next 1/3] net: sched: cls_api: add skip_sw counter

[Asbjørn Sloth Tønnesen]

Hi Jamal,

Thank you for the review.

On 2/15/24 17:39, Jamal Hadi Salim wrote:
> +Cc Vlad and Marcelo..
>
> On Thu, Feb 15, 2024 at 11:06 AM Asbjørn Sloth Tønnesen <ast@xxxxxxxxxxx> wrote:
> > Maintain a count of skip_sw filters.
> >
> > This counter is protected by the cb_lock, and is updated
> > at the same time as offloadcnt.
> >
> > Signed-off-by: Asbjørn Sloth Tønnesen <ast@xxxxxxxxxxx>
> > ---
> >   include/net/sch_generic.h | 1 +
> >   net/sched/cls_api.c       | 4 ++++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> > index 934fdb977551..46a63d1818a0 100644
> > --- a/include/net/sch_generic.h
> > +++ b/include/net/sch_generic.h
> > @@ -476,6 +476,7 @@ struct tcf_block {
> >          struct flow_block flow_block;
> >          struct list_head owner_list;
> >          bool keep_dst;
> > +       atomic_t skipswcnt; /* Number of skip_sw filters */
> >          atomic_t offloadcnt; /* Number of oddloaded filters */
>
> For your use case is skipswcnt ever going to be any different than offloadcnt?

No, we only use skip_sw filters, since we only use TC as a control path to
install skip_sw rules into hardware.

AFAICT offloadcnt is the sum of skip_sw filters, and filters with no flags which
have implicitly been offloaded.

The reason that I didn't just use offloadcnt, is that I'm not sure if it is
acceptable to treat implicitly offloaded rules without skip_sw, as if they were
explicitly skip_sw. It sounds reasonable, given that the filters without skip_* flags
shouldn't really care.

I tried to only trigger the TC bypass, in the cases that I was absolutely sure would
be safe as a first step.


> cheers,
> jamal
>
> >          unsigned int nooffloaddevcnt; /* Number of devs unable to do offload */
> >          unsigned int lockeddevcnt; /* Number of devs that require rtnl lock. */
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index ca5676b2668e..397c3d29659c 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -3483,6 +3483,8 @@ static void tcf_block_offload_inc(struct tcf_block *block, u32 *flags)
> >          if (*flags & TCA_CLS_FLAGS_IN_HW)
> >                  return;
> >          *flags |= TCA_CLS_FLAGS_IN_HW;
> > +       if (tc_skip_sw(*flags))
> > +               atomic_inc(&block->skipswcnt);
> >          atomic_inc(&block->offloadcnt);
> >   }
> >
> > @@ -3491,6 +3493,8 @@ static void tcf_block_offload_dec(struct tcf_block *block, u32 *flags)
> >          if (!(*flags & TCA_CLS_FLAGS_IN_HW))
> >                  return;
> >          *flags &= ~TCA_CLS_FLAGS_IN_HW;
> > +       if (tc_skip_sw(*flags))
> > +               atomic_dec(&block->skipswcnt);
> >          atomic_dec(&block->offloadcnt);
> >   }
> >
> > --
> > 2.43.0

--
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541