Re: [net-next v5 2/2] net: sched: support hash/classid/cpuid selecting tx queue

[Tonghao Zhang <xiangxia.m.yue@gmail.com>]

On Sat, Dec 25, 2021 at 3:26 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Tue, Dec 21, 2021 at 5:24 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> >
> > On Tue, Dec 21, 2021 at 1:57 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > >
> > > On Mon, Dec 20, 2021 at 4:39 AM <xiangxia.m.yue@gmail.com> wrote:
> > > >
> > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > >
> > > > This patch allows user to select queue_mapping, range
> > > > from A to B. And user can use skbhash, cgroup classid
> > > > and cpuid to select Tx queues. Then we can load balance
> > > > packets from A to B queue. The range is an unsigned 16bit
> > > > value in decimal format.
> > > >
> > > > $ tc filter ... action skbedit queue_mapping skbhash A B
> > > >
> > > > "skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
> > > > is enhanced with flags:
> > > > * SKBEDIT_F_TXQ_SKBHASH
> > > > * SKBEDIT_F_TXQ_CLASSID
> > > > * SKBEDIT_F_TXQ_CPUID
> > >
> > > Once again, you are enforcing policies in kernel, which is not good.
> > > Kernel should just set whatever you give to it, not selecting policies
> > > like a menu.
> > I agree that, but for tc/net-sched layer, there are a lot of
>
> If you agree, why still move on with this patch? Apparently
> you don't. ;)
>
> > networking policies, for example , cls_A, act_B, sch_C.
>
> You are justifying your logic by shifting the topics here.
Hi Cong,
I mean that if the "policies" are networking policies, I don't agree
with you, because
the TC(or net sched layer), are networking policies, right ? and this
patch enhances the original feature of TC.


> The qdisc algorithm is very different from your case, it is essentially
> hard, if not impossible, to completely move to user-space. Even if we
> had eBPF based Qdisc, its programmablility is still very limited.
> Your code is much much much easier, which is essentially one-line,
> hence you have to reason to compare it with this, nor you can even
> justify it.
I think we have talked about this in another thread. And other
maintainers comment on this.
> > > Any reason why you can't obtain these values in user-space?
> > Did you mean that we add this flags to iproute2 tools? This patch for
> > iproute2, is not post. If the kerenl patches are accepted, I will send
> > them.
>
> Nope, I mean you can for example, obtain the CPU ID in user-space
> and pass it directly to _current_ act_skbedit as it is.
This flag is passed from userspace. Do you mean the cpuid value should
be passed from userspace?
If so, this is not what we need. because we don't know which one cpu
the pod(containers) will use. and for skb-hash/classid
this helps us to pick the tx queue from a range. I don't understand
why this value should be passed from userspace. How ?
can you give me examples ?

BTW, this patch is the core of patchset.
> Thanks.



--
Best regards, Tonghao