Re: ulogd2 patch ping

[Jeremy Sowden <jeremy@azazel.net>]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On 2023-07-26, at 09:28:11 +0200, Pablo Neira Ayuso wrote:
> On Tue, Jul 25, 2023 at 08:11:28PM +0100, Jeremy Sowden wrote:
> > There is a ulogd2 patch of mine from the end of last that is still
> > under review in Patchwork:
> > 
> >   https://patchwork.ozlabs.org/project/netfilter-devel/patch/20221208222208.681865-1-jeremy@azazel.net/
> > 
> > It would be great to get a yea or nay.
> 
> What plugins are still IPv4-only in ulogd2?

ipfix, gprint, oprint and the SQL plug-ins all assume that input keys of
type ULOGD_RET_IPADDR are ipv4.  There is a separate ipv6 adddress type
(ULOGD_RET_IP6ADDR), but it is not used anywhere.

The question is what to do with ipv6 addresses on output, in particular
for the DB plug-ins where trying to write 128 bit values into possibly
32 bit columns might cause errors.  In the current version of the patch,
the gprint and oprint plug-ins are updated to output ipv6 addresses
correctly, the DB plug-ins replace the value with null, and I didn't fix
the ipfix plug-in.

As it happens, the mysql and pgsql schemas in doc/ should be fine, but
the sqlite3 one won't be.  My current thinking is to add a boolean
config setting to the SQL plug-ins to indicate whether the DB schema
can accommodate ipv6 addresses.

> Maybe add _IPV4 | _IPV6 flags to plugins hence it is possible to
> validate if user's stack is valid, otherwise bail out and provide a
> reason via logging?

The problem is that it isn't always possible to tell.  Consider a stack
with an NFLOG source.  The address family will depend on the family of
the table containing the rule outputting to the log, which is not
visible to ulogd.

> Regarding translation from network to host byte, I think it makes more
> sense to keep IPv4 addres in network byte, so filter and output
> plugings always expect them such way as you did in your patch?

Having revisited this work, I think my blithe assumption that I could
fix the handling of ipv6 addresses and the original endianness problem
in one patch may have been overly optimistic. :) In the next version, I
will separate the changes to keep ipv4 addresses in NBO from the ipv6
fixes.

Florian's observation about the ambiguity between real mapped ipv4
addresses and the synthetic ones created by this patch has prompted me
to revisit my approach to the ipv6 fixes.  My current thinking is to use
the `len` field of `struct ulogd_key` to keep track of the size of the
address: 4 = ipv4, 16 = ipv6.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]