Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 9 Sep 2014 13:01:13 +0200
From:      Luigi Rizzo <rizzo@iet.unipi.it>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        George Neville-Neil <gnn@neville-neil.com>, FreeBSD Current <current@freebsd.org>, Stefano Garzarella <stefanogarzarella@gmail.com>
Subject:   Re: RFC: please put back spare fields in struct ifnet (removed in svn 270870)
Message-ID:  <CA%2BhQ2%2BiuembLhrOLb8tqQ3Fv8b%2BNfs7uoPfvMSwp9ZOGCz99YA@mail.gmail.com>
In-Reply-To: <20140909103719.GB17059@glebius.int.ru>
References:  <CA%2BhQ2%2BiLGH%2Bh6asbQSES0P134_5KUgnt8Ve__UQgf9cwu_97eA@mail.gmail.com> <20140909103719.GB17059@glebius.int.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 9, 2014 at 12:37 PM, Gleb Smirnoff <glebius@freebsd.org> wrote:

>   Luigi,
>
> On Tue, Sep 09, 2014 at 12:13:42PM +0200, Luigi Rizzo wrote:
> L> svn 270870 removed all the if_*spare fields in struct ifnet.
> L> They are replaced with the following comment
> L>
> L> /*
> L>  * Spare fields to be added before branching a stable branch, so
> L>  * that structure can be enhanced without changing the kernel
> L>  * binary interface.
> L>  */
> L>
> L> =E2=80=8Bwhich leaves me a bit unhappy.
> L> Having a stable ABI is useful not only for stable branches,
> L> but also (I would say even more) with head, so people can
> L> run experimental code with limited modifications to the sources.
> L>
> L> Cases in point:
> L> - we used one spare field extensively when experimenting
> L>   with netmap, and being able to just build a module without having
> L>   to recompile the whole kernel was a big win.
> L> - we are developing some software GSO and again it was great to have
> L>   the spares in the tcpcb and in the ifnet so we could limit
> L>   modifications to headers used by multiple sources.
> L>
> L> I would kindly suggest to put the spares back.
> L> I can't see how they can possibly harm.
>
> The harm is obvious: someone commits code that _uses_ spare field
> without assigning it a new name. Spare field is a placeholder. Of
> course you can use it while you experiment. However, I don't see
> problem with patching your source tree where you experiment.
>

The problem with _not_ having spares is that you have to recompile
everything after patching the headers. With the spares, i could
make netmap a simple add-on kernel module with no dependencies.
Same on linux for what matters (there wasn't a spare there, but
nobody used ax25 and i could check and reuse it).

Of course you can easily emulate extension fields for stuff that
is not accessed frequently (tough a version number would help),
but there are cases when you do need the extra info on a per-packet
basis.


> The ABI plan for 'struct ifnet' is that the struct becomes
> opaque for device drivers. So its size and alignment no longer
> matters. Those who want to add new fields to struct ifnet,
> would also need to add accessor methods. Bits of this plan
> are already committed by Marcel, but its only first step.
>

=E2=80=8Bspare fields <-> spare accessors=E2=80=8B


> I'm afraid that if fields are there back, the situation that
> happened with netmap (use of spare field) would repeat.
>

=E2=80=8Bthe spare pointer used by netmap was clearly indicated by a commen=
t,
and giving it a dedicated name instead of if_pspare[0] was
expected to happen (hopefully together with a __FreeBSD_version bump,
which has not happened).

I am not worried that the name change was missed when deleting if_pspare[].
Mistakes happen and the error was promptly corrected (apart from the
version bump).

What worries me is losing some flexibility in dealing with
out-of-tree kernel modules.

=E2=80=8Bcheers
luigi=E2=80=8B



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BhQ2%2BiuembLhrOLb8tqQ3Fv8b%2BNfs7uoPfvMSwp9ZOGCz99YA>