Re: Initializing a device

[Guy Harris <guy () alum mit edu>]

On Jan 5, 2012, at 1:40 AM, Akos Vandra wrote:

> I browsed through the code of pcap_open_live, and pcap_set_promisc,
> and related stuff, and I think that now I understand how this works.
>
> However in my opinion, the way parameter passing is implemented breaks
> the principle of modularity.
>
> iface here = pcap_canusb, pcap_usbmon, etc.
> What is the correct term for these?

There isn't one.  I've used "device-type module" later in this message.  "iface" is close to "interface", and that 
could be confused with "interface" in the sense of a network interface, on which you might do capture.

> I keep using iface and module interleavedly in my mails...
>
> If - let's say - my interface (canusb) needs a parameter for the baud
> rate, and another interface, let's say a more low-level radio iface
> needs parameters like frequency, channel, modulation (ASK or FSK),
> then to fully support these capture ifaces, we would need to add a lot
> of parameters to pcap_open_live,

No, we wouldn't need to (and we wouldn't ever do so, as that'd break source and binary compatibility).

You browsed through pcap_open_live(), so presumably you've seen that it's just

pcap_t *
pcap_open_live(const char *source, int snaplen, int promisc, int to_ms, char *errbuf)
{
        pcap_t *p;
        int status;

        p = pcap_create(source, errbuf);
        if (p == NULL)
                return (NULL);
        status = pcap_set_snaplen(p, snaplen);
        if (status < 0)
                goto fail;
        status = pcap_set_promisc(p, promisc);
        if (status < 0)
                goto fail;
        status = pcap_set_timeout(p, to_ms);
        if (status < 0)
                goto fail;

        [some code to make the backwards-compatibility stuff work a little better elided]

        status = pcap_activate(p);
        if (status < 0)
                goto fail;
        return (p);
fail:
        [error-message-generation-code elided]

        pcap_close(p);
        return (NULL);
}

I.e., that it's a wrapper around pcap_create() and pcap_activate().  It exists so that programs written using the old 
pcap_open_live() API will continue to compile and run.

Programs that need to use *any* capabilities not provided by pcap_open_live(), such as setting the kernel capture 
buffer size, setting monitor mode on 802.11 interfaces, or setting the time stamp type, must use the newer APIs - 
pcap_create(), the appropriate setter routines, and pcap_activate().

> and a lot of API functions (like
> pcap_set_modulation, pcap_set_channel) which are used by a single
> iface.
>
> That is - in my opinion - bad.

The current API was chosen during a discussion on the tcpdump-workers mailing list back in 2008.

The thread can be found in

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/

We ended up going with something along the lines of Michael Richardson's suggestion:

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2830

"I.e. we should instead have a new pcap_open() call, and then we should
have a serious of calls that set various options or ask for various
things based upon that handle."

> My suggestion for a solutoin - which is a *lot* of work -, but will
> add a nice functionality to libpcap:
>
> Please view this from a higher level, if you guys like it we might
> think about data structures, etc later, I have ideas about that too,
> but they are quite immature yet.
>
> Add a new _op command list_parameters, which returns a list of the
> parameters that it needs with data about: parameter name, type,
> default value. If the _op is unset, or returns null, it means the
> iface doesn't need any (backw compatibility).
>
> Add a few new API calls to create a list of parameters, and set values
> in it. These functions check if the correct type of argument is given.
> params_create - returns a handle
> params_set_uint8 - sets an uint value, etc.
>
> Then pass this parameters structure to the activate_op, and it can do
> the activation based on these parameters.
>
> Now if we add a new parameter, there is no need for new API calls,
> just add a new entry to the param_list.

Something along those lines was proposed in that thread:

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2818

and that proposal evolved:

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2819

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2820

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2822

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2823

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2824

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2826

and then Michael Richardson said

        http://thread.gmane.org/gmane.network.tcpdump.devel/2800/focus=2828

which, shortly afterwards, got us moving in the direction of his suggestion.

An API pattern based on a list of parameters is more complicated than the current API pattern, as you have to create 
and manipulate the list.

One disadvantage of the current API pattern is that it can't handle new parameters added by new device-type modules 
without changing the core API.

Another small one is that if a program *could* work if a given parameter can't be set, by, say, not offering the option 
to set it, but *should* offer that option if it's available - a classic autoconf example, i.e. check whether 
pcap_set_XXX() is available and set a #define HAVE_PCAP_SET_XXX, and use #ifdefs to call it only if it's available - 
you could potentially avoid autoconf if the options are identified by #define names, by just #ifdeffing on the #define 
name.

One disadvantage of changing the API pattern is that programs that would use the new API's capabilities will have to 
add more autoconf stuff to check for the new new API and use it rather than using the new 
(pcap_create()/pcap_set_XXX()/pcap_activate()) API if available or the old (pcap_open_live()) API if not.  If we're 
going to change the API pattern again, let's make sure it's the last time we do that....

> This would be very handy for UIs too, as they could implement a
> generic capture interface options dialog, and use the list returned by
> the param_list_op to populate it with widgets or controls.

Yes, although a UI might still handle some options specially, e.g. if, as, and when we add an 802.11 channel option, it 
might only be made available if monitor mode was selected, so a GUI should either hide or gray out the channel option 
if monitor mode wasn't selected.
 
> I think the whole decision has one very sensitive part: As this
> architecture mainly eases implementation of different kinds of ifaces
> which are *not* based on ethernet, like my can device, or an i2c
> device, or a generic radio device, or whatever comes to mind, the main
> question is if it's libpcap's goal to support these.

libpcap's goal from Day One has been to support arbitrary network interface types - one interface type that has had 
special-parameter support for a while is 802.11, for which we have the "monitor mode" flag.-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.