Re: `among' match

On Wed, 3 Sep 2003, Bart De Schuymer wrote:

> On Wednesday 03 September 2003 09:32, Grzegorz Borowiak wrote:
> > On Wed, 3 Sep 2003, Bart De Schuymer wrote:
> > > Perhaps it's better to make 2 matches out of it, one for source, one for
> > > destination. That will save space if you only want source stuff.
> >
> > These matches would have most of their code shared and would differ only
> > slightly. So we would have two pieces of code, each only slightly less
> > than present one piece of code.
>
> It should be possible to have both matches in one module. ebt_nat userspace is
> like that. Kernel space should be possible too, the only problem is the name
> of the matches, since currently ebtables tries to insmod ebt_matchname, which
> would fail if the match names are different from the kernel file containing
> both. But this is a userspace problem that can be fixed.

Still I'm not convinced if benefits are worth making such tricks.

There's no point in having two separate kernel modules. They would
duplicate a major piece of code. ebt_arp also could be splitted into
several modules, but it isn't and it - IMHO - shouldn't be. ebt_among as
well: could, but shouldn't.

The only practical benefit from splitting I see is some minor speedup
here:

static int ebt_filter_among(const struct sk_buff *skb,
   const struct net_device *in, const struct net_device *out,
   const void *data, unsigned int datalen)
{
        struct ebt_among_info *info = (struct ebt_among_info *) data;

        const char *dmac, *smac;
        if (info->bitmask & EBT_AMONG_SRC) {
                smac = skb->mac.ethernet->h_source;
                if (!ebt_mac_wormhash_contains(&info->wh_src, smac))
                        return EBT_NOMATCH;
        }

        if (info->bitmask & EBT_AMONG_DST) {
                dmac = skb->mac.ethernet->h_dest;
                if (!ebt_mac_wormhash_contains(&info->wh_dst, dmac))
                        return EBT_NOMATCH;
        }

        return EBT_MATCH;
}

These two and'ing of bitmasks could have been omitted.

For the folks who need it, maybe I should make a
CONFIG_BRIDGE_EBT_AMONG_SRCONLY bool config option?

> > > What would also be very nice, I think, is the following extension of your
> > > module: have MAC address - IP address correspondences. So one could have
> > > one anti-spoofing rule with lots of MAC-IP correspondences.
> >
> > OK, but such a match would be complicated in its way. It should check a
> > protocol (IPV4 or ARP), then extract IP address dependently on protocol.
> > So implementation of this would take some time for me, about 2 weeks
> > (primarily, I'd like to implement some ebt features that I need, so they
> > have higher priority).
> >
> > Maybe should I make this extended match as a separate module?
>
> Yes, it would be a different module.

OK.

> > Does ebtables interface allow to use variable-sized info structures? This
> > would be necessary to achieve this.
>
> Yeah it does. In userspace, you just need to update the size and possibly the
> pointer to the struct ebt_entry_match. The parse function has as argument
> "struct ebt_entry_match **match", so you can change the pointer to the
> ebt_entry_match if f.e. you need more memory for the match data.

So:

allocate new memory
change the pointer pointed to by that double pointer
copy everything from old ebt_entry_match to the new one, byte-to-byte
change the match_size field
free the memory at the old pointer

Did I get the idea?

If so - OK.

-- 
Grzesław


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf