On Fri, Jul 13, 2018 at 6:04 AM Marcelo Ricardo Leitner
<marcelo.leit...@gmail.com> wrote:
>
> On Thu, Jul 12, 2018 at 11:05:45PM -0700, Cong Wang wrote:
> > On Wed, Jul 11, 2018 at 12:33 PM Marcelo Ricardo Leitner
> > <marcelo.leit...@gmail.com> wrote:
> > >
> > > On Tue, Jul 10, 2018 at 07:25:53PM -0700, Cong Wang wrote:
> > > > On Mon, Jul 9, 2018 at 2:40 PM Marcelo Ricardo Leitner
> > > > <marcelo.leit...@gmail.com> wrote:
> > > > >
> > > > > On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
> > > > > > Changing TC_PRIO_MAX from 15 to 63 risks breaking backward
> > > > > > compatibility
> > > > > > with applications.
> > > > >
> > > > > If done, it needs to be done carefully, indeed. I don't know if it's
> > > > > doable, neither I know how hard is your requirement for 64 different
> > > > > priorities.
> > > >
> > > > struct tc_prio_qopt {
> > > > int bands; /* Number of bands */
> > > > __u8 priomap[TC_PRIO_MAX+1]; /* Map: logical priority ->
> > > > PRIO band */
> > > > };
> > > >
> > > > How would you do it carefully?
> > >
> > > quick shot, multiplex v1 and v2 formats based on bands and sizeof():
> > >
> > > #define TCQ_PRIO_BANDS_V1 16
> > > #define TCQ_PRIO_BANDS_V2 64
> > > #define TC_PRIO_MAX_V2 64
> > >
> > > struct tc_prio_qopt_v2 {
> > > int bands; /* Number of bands */
> > > __u8 priomap[TC_PRIO_MAX_V2+1]; /* Map: logical priority ->
> > > PRIO band */
> > > };
> > >
> >
> > Good try, but:
> >
> > 1. You don't take padding into account, although the difference
> > between 16 and 64 is big here. If it were 16 and 20, almost certainly
> > wouldn't work.
>
> It still would work, no matter how much padding you have, as currently
> you can't use more than 3 bands.
I am lost.
With your proposal above, you have 16 bands for V1 and 64 bands
for V2, where does 3 come from???
>
> >
> > 2. What if I compile a new iproute2 on an old kernel? The iproute2
> > will use V2, while old kernel has no knowledge of V2, so it only
> > copies a part of V2 in the end....
>
> Yes, and that's not a problem:
> - Either bands is > 3 and it will return EINVAL, protecting from
> reading beyond the buffer.
> - Or 2 <= bands <= 3 and it will handle it as a _v1 struct, and use
> only the original size.
Again why 3 not 16 or 64 ??
Also, why does an old kernel has the logic in its binary to determine
this?
>
> iproute2 (or other app) may still use _v1 if it wants, btw.
Yes, old iproute2 must still have v1, what's point? Are you
suggesting new iproute2 should still have v1 after you propose
v1 and v2 for kernel?
I must seriously miss something. Please help.
Thanks!
