tcpdump mailing list archives
Re: Initializing a device
From: Akos Vandra <axos88 () gmail com>
Date: Fri, 6 Jan 2012 16:47:09 +0100
Hi! I have read through the threads you mentioned, it's nice to see that something similar has already been suggested, I would like to present a few new arguments in favor of them. On 6 January 2012 12:15, Guy Harris <guy () alum mit edu> wrote:
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.
Okay, we should settle on some name like device-type module, or capture module before talking about this gets too confusing - you chose. :)
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).
...............
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. "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.
Yeah, this is what I was trying to point out, and what I find to be quite a barrier for scalability of libpcap. I will be comparing libpcap to wireshark in the next few lines, as they are quite close to eachother, and I really love the architecture of wireshark. The best thing would be if it was possible to add new capture modules (aka device-type modules) just by writing a pcap-*.c file, and adding a few lines to the makefile and autoconf. As you said the only way to add new parameters with the current API is to extend it with a new setter function, which may or may not be called by the user program to set the parameters new value as needed. Right now the API works by setting the right member field of the opts structure for each parameter, so in order to add a new parameter, the opts structure must be extended as well. Another approach would be for the API to call function pointers (if not NULL) from the pcap structure, which would avoid an opts structure, and linking in code from the capture modules themselves, but then we would need to extend the pcap structure itself with these function pointers, because we cannot link in the setter code from the capture modules (which might not be supported on some platforms or configurations). The main problem here is that in any case in order to add a new parameter, either the pcap or the opts structure needs to be extended, and probably only one capture module will be using that member function pointer, or opts member field. IMHO With many capture modules this might get out of hand, and anyways the need to even to extend, not modify the API in order to support a new module is not a good shot. Another reason why this is not a good approach: Let's get wireshark in the picture. Let's say the user selected a canusb device. The only way for wireshark to know what parameters (ex. baudrate) the canusb device needs is if wireshark knows how the canusb device works. This is bad. IMHO one of the main goals of libpcap would be to hide how the capture device actually works from the user application, so that it can use a device-independent way of getting packets. Even if wireshark would magically know that it needs to request a baudrate from the user, and does, it would need to know that there is an API call names pcap_set_baud(pcap_t*, int) used for that. And this is where the most important problem gets in the picture. Let's say that now libpcap is updated on the system, and a new capture module, "usbradio" is added, which needs "frequency" and "channel" parameters. Even if wireshark magically knows (maybe by querying libpcap for that) that it needs to request these parameters from the user, there is ABSOLUTELY NO WAY for it to call pcap_set_frequency() and pcap_set_radiochannel() to set those parameters, as wireshark was not linked to call those methods! So in order to add support for a new type of device, wireshark needs to be patched and recompiled, which breaks the principle of modularity. (In order to extend libpcap you need to extend wireshark). In my solution, no new API calls are added, and wireshark would auto-magically support any new devices. It would work this way: - wireshark queries libpcap for available capture devices / interfaces - user select one device, and asks wireshark what options can be set - wireshark calls pcap_create(), which sets op_activate, and op_getoptlist, op_setparam, op_getparam - wireshark calls pcap_get_optlist(), which is forwarded to the capture modules op_getoptlist. This returns a list of tuples, with option's <name, id, TYPE, default value> - wireshark presents an dialog generated on-the-fly using the getoptlist. - user sets some parameters, chooses ok - wireshark repeadetly calls pcap_setparam_int(id, value) or pcap_setparam_bool(id, value), or whatever, which checks for parameter type safety, and calls the capture module's pcap_setparam(id, &value). (Dereferencing will be safe as the API already checked for type safety. This idea is from the wireshark API, a few basic types will be supported, like UINT, INT, BOOL, ENUM, etc. - wireshark calls pcap_activate(). Advantages comparing to my former suggestion are that there is no complication for maintiaining a parameter list. The API is static, no need to extend it in order to add new parameters to a capture module. Wireshark (or some other user application) can use new interfaces without any logic added to them, if new parameters are present, it can either ask them from the user, or use the default values. If a capture module does not need any parameters, the op_getoptlist may simply be NULL. the pcap strucure's opts member could be removed, but better be deprecated for backw compatibility (user application shouldn't have poked around it anyways). Those few modules like the 802.11 which used it can be rewritten to set the parameters with the new API, user applications would never see the difference. Advantages comparing to the discussion on the list would be that there is no central repository for the parameters, so each capture module can maintain it's own list. (I would add a version number here as well, it might be needed where a user application uses a specific capture module, and does know how it works, and wants to set its parameters automatically) - This allows all capture interfaces to be contained in a single file (except registration! - see later). Parameter values can be set even by name or id. The first one is more useful for human-inputs and the latter for automatic control of parameters. Even though it does not change the API, I would also change the way the best capture module is chosen by pcap_open. Right now there is a bunch of calls to strstr checking for a magic string. In order to add support for a new module, this function must be extended by adding another strstr call, and calling the appropriate pcap_xxx_create function if the test succeeds. I had problem with this when adding the "canusb" capture device. There was already a "can" module, and I added my strstr after the "can" check, so libpcap never got there (as the test against "can" succeded). If you ask me, I would either trust the pcap_xxx_create function with this test, and if it suspects the device is not for that module, return NULL. Then just call the pcap_xxx_create functions until one of them returns a non-NULL pointer, which means its test succeded, and it succeded creating the pcap device, or until it runs out of capture module pcap_xxx_create()-s to call. I would go further, by adding a pcap_register_capture_module(pcap_t (*constructor)(char*device, char*err)) call, (constructor is what pcap_xxx_create is now) which would be called once on initialization of the pcap library, it would load all the possible capture modules into an array or list, and then it would iterate over the list and call the pcap_xxx_cretate funcitons one-by-one, until the list ends, or one of them returns a valid pcap_t structure. I would go even further (i suspect this will be denied :) by putting all the capture modules in their own mini-library, which would be loaded by the libpcap initialization function. It would get the contents of a magic directory containing all the capture module libraries, dynamically loading each one of them, and calling the pcap_capture_module_init function, which in turns calls back the pcap_register_capture_module with the appropriate constructor functions. This way all capture modules would truly be contained only in a single source file, and a new capture module might be added without even recompiling libpcap itself, but just copying the appropriate mini-capture-module-library in the magic directory. Wireshark works in a very similar way, except that instead of using dynamic linking of these libraries it uses a script to generate the function which calls all register functions in the submodules, and links them at linking time. (that's a repetition I don't know how to avoid) Disclaimer: I am usually the guy with a lot ideas (some brilliant and some really stupid), but usually can't figure out which one is from which category :). This is what you guys are here for, you probably have a *lot* more experience than me, especially with opensource projects, writing portable software, and software design. These are my two cents in on what I think would be awesome, so please don't get offended that I kinda want to change everything, God forbid me to intend to say that what is now is not good. Also my description of the way it works still needs a few rounds of refining, but I guess it's enough for you to understand what am I thinking of. Regards, Ákos Vandra - This is the tcpdump-workers list. Visit https://cod.sandelman.ca/ to unsubscribe.
Current thread:
- Initializing a device Akos Vandra (Jan 04)
- Re: Initializing a device Sam Roberts (Jan 04)
- Re: Initializing a device Guy Harris (Jan 04)
- Re: Initializing a device Akos Vandra (Jan 05)
- Re: Initializing a device Guy Harris (Jan 06)
- Re: Initializing a device Akos Vandra (Jan 06)
- Re: Initializing a device Jakub Zawadzki (Jan 06)
- Re: Initializing a device Akos Vandra (Jan 06)
- Re: Initializing a device Guy Harris (Jan 11)
- Re: Initializing a device Akos Vandra (Jan 12)
- Re: Initializing a device Guy Harris (Jan 12)
- Re: Initializing a device Akos Vandra (Jan 12)
- Re: Initializing a device Akos Vandra (Jan 17)
- Re: Initializing a device Akos Vandra (Jan 05)
- Re: Initializing a device Guy Harris (Jan 06)