Script for updating "oui.c"

[sgeto]

[sgeto]

Almost there...

[fxlb]

Hi, Some remarks:

• update-oui-database.sh is licensed under GPL, not sure we take it.
• update-oui-database.sh is not executable.
• Striped size of tcpdump increase by 66% (e.g. 1,2M->2.0M on Debian Jessie). Perhaps problem for embedded systems. Make update optional ?
• After running update-oui-database.sh (I need to install gawk, my system has mawk by default), the oui.c file is only 3220 bytes.
• Your branch is 2 commits behind the-tcpdump-group:master.

[infrastation]

Also it would help to have one clean commit instead of many draft changes.

[sgeto]

@fxlb

update-oui-database.sh is licensed under GPL

So is config.guess and config.sub. I believe that tcpdump's source is licensed under a BSD-style license, not so much the scripts to build it. And this script isn't even meant to be used by a developer or user. Only by tcpdump maintainers', and probably only once a year or so.

update-oui-database.sh is not executable

Sorry. You're right. I keep forgetting that. Executable bits aren't really a thing one my main OS. I will fix it that.

Make update optional ?

I guess there are cases where increases file size could be a problem. So this could be done. Although I also think that compiling these entries in should be the default. My NanoStation Loco M2 from 2011 for example is barely complaining, and its total free space is 8 MiB (OpenWrt uses about 4 MiB of that).

Also, I noticed that prefixing every entry with 0x adds about 0.4 MiB to the total file size (stripped). Using a more efficient method of marking these as hex is probable in order too.

After running update-oui-database.sh (I need to install gawk, my system has mawk by default), the oui.c file is only 3220 bytes

oui.txt (from http://standards-oui.ieee.org/oui.txt) has Windows line endings by default. 😆 😆 I didn't notice that. As convenient as as that may be for me, it wasn't their smartest move. You need to convert them (i.e the script should probably do that for you). I'm on it. And then try it mawk. Maybe it works.

@infrastation

Also it would help to have one clean commit instead of many draft changes.

I will squash them into a single commit once I come after some of the points mentioned.

@gvanem Python and I aren't on out best terms. The dry-run option is great though 👍

[mcr]

if you are talking about the 0x in the oui_values table in oui.c, then the size of the .c file has no relationship to the resulting object file.

[mcr]

There is also the PEN table in oui.c, which would be nice to update. It might be that a better policy would be to install a file into /usr/share that has the list. I hate to parse that file each time tcpdump runs, so it should at least be on demand... maybe we can mmap() it read only. At least that eliminates the hit for small systems (just don't ship the file). Or maybe compile it has as now, with #ifdef SMALL or something.

[sgeto]

I agree, an #ifdef SMALL or something in oui.c and a note somewhere should be enough. There could also be an option for that in the configure script.

There is also the PEN table in oui.c, which would be nice to update.

The OUI database is significantly smaller than the PEN table. This would add 51000+ entries.

[gvanem]

@sgeto Python and I aren't on out best terms. The dry-run option is great though

I deleted that comment since you d/l it since it failed in Python3 (it still have some Unicode issue). But here it is again with a cached tweak: make-oui.py.txt

I.e. it doesn't download again if oui.txt is in curr-dir. (drop the .txt extension again and run python2 make-oui.py)

[sgeto]

@gvanem whoa the script is fast! (It took around a second)

This is clever f.write (" { 0xFFFFFF, NULL } /* Since XEROX CORPORATION has value 0, use this */\n };\n")

but the script adds only "netdissect.h" when oui.c needs

#include <netdissect-stdinc.h>
#include "netdissect.h"
#include "oui.h"

It also seems to overrides already existing content in oui.c (PEN table, copyright notice, includes etc). I spend quite some time on mine to make sure it never does.

[gvanem]

What is PEN table? Okay, it's trivial to add the other headers (I've not actually built tcpdump with this oui.c).

[sgeto]

What is PEN table?

the other structure in oui,c that begins with

/*
 * SMI Network Management Private Enterprise Codes for organizations.
 *
 * XXX - these also appear in FreeRadius dictionary files, with items such
 * as
 *
 *	VENDOR          Cisco           9
 *
 * List taken from Ethereal's epan/sminmpec.c.
 */

[sgeto]

I'm pretty happy with how the PR is right now. So what would it take to get this baby on the road? If you have suggestions or really want me to squeeze these commits on one, let me know.

[gvanem]

I'm still tinkering with my make-oui.py script. But have some problems understanding how all the oui.h entries (like #define OUI_IANA) should be used/generated. Are they not?

Having all these OUI_xx in oui.h is IMHO ugly. Should we rather make some generated:

oui_val_to_name()
oui_name_to_val()

functions? (ref. pcap_datalink_val_to_name).

Edit: I've updated my .py script for a smi_values[] array too: https://gist.github.com/gvanem/1643c946fb2395b6c8a05c3ec8904e13

Compiles okay with gcc/clang-cl now. But MSVC says:

oui-generated.c : fatal error C1128: number of sections exceeded object file format limit: 
compile with /bigobj

Besides the size of windump.exe jumped from 1.522.688 to 3.819.008 bytes!

[infrastation]

As a matter of style, for a change of this scope 1 commit would be better than 12 commits trying to amend one another.

[sgeto]

@infrastation done

[sgeto]

@gvanem

But have some problems understanding how all the oui.h entries (like #define OUI_IANA) should be used/generated. Are they not?

Will this change, these definitions are only around for print-lldp.c and print-llc.c.

Having all these OUI_xx in oui.h is IMHO ugly.

I agree but just leave them there. Another option would be to replace these macros in print-lldp.c and print-llc.c with with their actual value since they will probably stay the same forever anyways.

oui-generated.c : fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj

Have not build this branch with MSVC yet. If MSVC knows that the resulting object file is exceeding the object file format limit (the have a term for everything, don't they?), why wouldn't the compiler add that flag by itself? That's just backwards. Does the error go away when you add /bigobj?

Besides the size of windump.exe jumped from 1.522.688 to 3.819.008 bytes!

oui.txt is a big list. An alternative would be to make the list optional or to make tcpdump read a plain oui.txt at runtime. See @mcr 's comment on that. The latter will require quite some coding. Whether this feature should be around is a more important question than the increasing filesize. And because it's 2017, that question should also be a rhetorical one.

[ValZapod]

I will just remind you that wireshark uses scripts to update all of it (SMI as well) once in 7 days. Like that https://code.wireshark.org/review/c/37293/

[fxlb]

Need to study memory and size before/after adding a complete oui file. I think we need to keep tcpdump small. Make this optional?

[mcr]

Alternatively, I have the OUIs as a reverse DNS lookup:

tilapia-[/etc/domain/sandelman.ca] mcr 10007 %dig +short 1.0.0.5.c.b.1.0.0.ethermap.sandelman.ca. txt
"OpenRB.com, Direct SIA"

I don't really want to put this in my zone. It could go into tcpdump.net if desired.

[infrastation]

@fxlb, did you have a chance to see what effect this change would have on performance and memory footprint?

[fxlb]

did you have a chance to see what effect this change would have on performance and memory footprint?

Stripped size of tcpdump increases from 1.6M -> 2.2M on Debian Bullseye. No time for other studies.

(This PR updates 107 files : problem!)

[infrastation]

Most of the changes are not related to OUI printing, possibly a git rebase went wrong at some point. The proposed change would need to be extracted into a small commit before it can be considered again.

[infrastation]

The changes that belong to the commit are:

• update-oui-database.sh (a new file)
• Makefile.in (one line added for the above)
• oui.h (some lines removed)
• oui.c (many lines added)
• configure (support for --disable-oui-database to define NOOUIDB)

The original changes to configure.ac seem to be lost and would need to be recovered.