netdev
[Top] [All Lists]

Re: [PATCH] Prefix List patch against 2.5.70

To: krkumar@xxxxxxxxxx
Subject: Re: [PATCH] Prefix List patch against 2.5.70
From: "David S. Miller" <davem@xxxxxxxxxx>
Date: Fri, 30 May 2003 23:32:29 -0700 (PDT)
Cc: kuznet@xxxxxxxxxxxxx, netdev@xxxxxxxxxxx, linux-net@xxxxxxxxxxxxxxx
In-reply-to: <3ED80230.2030508@xxxxxxxxxx>
References: <3ED80230.2030508@xxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
   From: Krishna Kumar <krkumar@xxxxxxxxxx>
   Date: Fri, 30 May 2003 18:15:28 -0700

This code is so gross..
   
   +/* prefix list returned to user space in this structure */
   +struct plist_user_info {
   +    char name[IFNAMSIZ];            /* interface name */
   +    int ifindex;                    /* interface index */

Just as Yoshfuji said, name is determinable with ifindex alone.
Don't duplicate information.

   +    struct var_plist_user_info {    /* multiple elements */
   +            char flags[3];          /* router advertised flags */

BARF

   +#ifdef CONFIG_IPV6_PREFIXLIST
   +    BUG_TRAP(list_empty(&idev->prefix_list) == 1);
   +#endif

list_empty() returns a boolean, which for true is not necessarily the
integer value "1".  This will therefore never trigger when it should
on cpus which use "-1" as the boolean value true.

You have all of this complexity which duplicates work made by
the routing code to keep track of these kinds of entities.

Your "takes a long time to walk the routes" argument is NULL
and void, what if I have 6,000 VLAN interfaces configured up
speaking ipv6?  This is not a hypothetical situation, I know
people who do this.

If you're worried about search complexity, you can keep the
"interesting" routes on a special linked list to avoid having to walk
all of the routes.  These prefix things really do seem like route
attributes.

That is 1,000 times cleaner than this thing you are showing us.

<Prev in Thread] Current Thread [Next in Thread>