Re: kni: check abi version between kmod and lib

[Stephen Coleman <omegacoleman@gmail.com>]

thanks for your replies

I'm aware that kernel guidelines propose ascending ioctl numbers to
max out compatibility, but this will not work with dpdk, especially
our case here.

If you look into kni_net.c you'll see the module is actually
internally depending on the memory layout of mbuf and a few other
structs, you will need to change ioctl numbers if those change, and
that's very implicit and requires extra effort. Plus the compatibility
is almost impossible to maintain across dpdk releases, as the module
won't know which version of mbuf layout it is working with.

In short, rte_kni.ko is part of dpdk rather than part of kernel, and
different parts of different dpdk releases do not work together -- so
we reject them early in the first before it make a disaster.

p.s. working on v3 to fix code format issues
p.p.s. forgot to 'reply all' last time, sorry for the duplication


>
>
> Stephen Hemminger <stephen@networkplumber.org> writes:
>
> > On Thu, 21 Apr 2022 11:40:00 -0400
> > Ray Kinsella <mdr@ashroe.eu> wrote:
> >
> >> Stephen Hemminger <stephen@networkplumber.org> writes:
> >>
> >> > On Thu, 21 Apr 2022 12:38:26 +0800
> >> > Stephen Coleman <omegacoleman@gmail.com> wrote:
> >> >
> >> >> KNI ioctl functions copy data from userspace lib, and this interface
> >> >> of kmod is not compatible indeed. If the user use incompatible rte_kni.ko
> >> >> bad things happen: sometimes various fields contain garbage value,
> >> >> sometimes it cause a kmod soft lockup.
> >> >>
> >> >> Some common distros ship their own rte_kni.ko, so this is likely to
> >> >> happen.
> >> >>
> >> >> This patch add abi version checking between userland lib and kmod so
> >> >> that:
> >> >>
> >> >> * if kmod ioctl got a wrong abi magic, it refuse to go on
> >> >> * if userland lib, probed a wrong abi version via newly added ioctl, it
> >> >>   also refuse to go on
> >> >>
> >> >> Bugzilla ID: 998
> >> >
> >> >
> >> > Kernel API's are supposed to be 99% stable.
> >> > If this driver was playing by the upstream kernel rules this would not
> >> > have happened.
> >>
> >> Well look, it is out-of-tree and never likely to be in-tree, so those
> >> rules don't apply. Making sure the ABI doesn't change during the ABI
> >> stablity period, should be good enough?
> >>
> >
> > I think if KNI changes, it should just add more ioctl numbers and
> > be compatible, it is not that hard.
>
> True, fair point, I am unsure what that buys us though. My thinking was
> that we should be doing the minimal amount of work on KNI, and directing
> people to use upstream alternatives where possible.
>
> For me minimizing means DPDK ABI alignment. However I see your point,
> let KNI maintain it own ABI versioning independent of DPDK, with
> stricter kernel-like guarantees is probably not much more work.
>
> --
> Regards, Ray K