Date: Fri, 17 Feb 2012 09:23:42 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Marcel Moolenaar <marcel@xcllnt.net> Cc: net@freebsd.org Subject: Re: Abstracting struct ifnet Message-ID: <20120217082342.GA15346@onelab2.iet.unipi.it> In-Reply-To: <338757D1-6B1E-49CF-983F-5D5851066FD3@xcllnt.net> References: <338757D1-6B1E-49CF-983F-5D5851066FD3@xcllnt.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Feb 16, 2012 at 08:16:22PM -0800, Marcel Moolenaar wrote:
> All,
>
> Juniper is in the final phases of creating a clean separation
> between FreeBSD and Junos, so as to make upgrades of FreeBSD
> easier. This also allows Juniper to track -current and be more
> active FreeBSD contributors.
>
> To that end, we have a short-term and hopefully short-lived
> problem to solve, which is the ability to use FreeBSD's network
> drivers against the Junos network stack. As some may know, the
> Junos network stack has split up struct ifnet into a physical
> and logical component, called ifdev and iflogical.
>
> We've tried a few approaches to bridge the gap between ifnet
> on the one hand and ifdev and iflogical on the other and found
> that abstracting ifnet and using accessor functions is the
> best way to allow us to use FreeBSD drivers with the Junos
> network stack, while retaining the ability to use them with
> the FreeBSD stack.
>
> FreeBSD is also looking at breaking up ifnet and with that in
> mind, I was wondering if there would be any resistance to
> changing network drivers to use accessor functions or macros
> instead of direct pointer dereferences?
>
> For example, do something like:
>
> Index: if_fxp.c
> ===================================================================
> --- if_fxp.c (revision 231178)
> +++ if_fxp.c (working copy)
> @@ -823,13 +823,14 @@
> }
>
> if_initname(ifp, device_get_name(dev), device_get_unit(dev));
> - ifp->if_init = fxp_init;
> - ifp->if_softc = sc;
> - ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> - ifp->if_ioctl = fxp_ioctl;
> - ifp->if_start = fxp_start;
> + if_set_init(ifp, fxp_init);
> + if_set_softc(ifp, sc);
> + if_set_flags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST, 0);
> + if_set_ioctl(ifp, fxp_ioctl);
> + if_set_start(ifp, fxp_start);
>
> - ifp->if_capabilities = ifp->if_capenable = 0;
> + if_set_capabilities(ifp, 0);
> + if_set_capenable(ifp, 0);
>
> /* Enable checksum offload/TSO for 82550 or better chips */
> if (sc->flags & FXP_FLAG_EXT_RFA) {
>
> Such a scheme, while initially touching a lot of driver,
> would make it easier to break up ifnet *and* also make it
> easier to hide ABI/API changes from driver vendors (esp.
> when the accessor functions are non-inlined functions and
> not macros or inlines). This is particularly useful for
> Juniper, where we have worked towards network stacks as
> (pre-)loadable modules so as to help with migration and
> validation.
>
> Thoughts, feedback and suggestion are welcome,
I do like the idea, but the amount of changes will be massive
(see below). The thing that worries me the most is that it
will introduce huge changes between different releases, unless
we backport the accessors (while keeping the underlying struct ifnet
frozen so we preserve the kernel ABI).
To count the number of lines affected, in shell:
$ for i in if_addrhead if_index if_multiaddrs if_input if_output \
if_snd if_addr_lock if_init if_softc if_flags if_ioctl if_start \
if_capenable if_addr if_refcount if_fib if_drv_flags \
if_data if_index_reserved if_l2com if_link if_xname if_dname
do
printf "%-20s" $i; grep -r $i ~/FreeBSD/head/sys | wc -l
done
And you should get something like this
if_addrhead 111
if_index 231
if_multiaddrs 144
if_input 184
if_output 112
if_snd 976
if_addr_lock 12
if_init 356
if_softc 1462
if_flags 1785
if_ioctl 259
if_start 194
if_capenable 1103
if_addr 324
if_refcount 6
if_fib 27
if_drv_flags 1987
if_data 202
if_index_reserved 2
if_l2com 346
if_link 187
if_xname 431
if_dname 20
cheers
luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120217082342.GA15346>