Date: Thu, 20 Oct 2005 10:20:57 +1300 From: Andrew Thompson <thompsa@freebsd.org> To: Andre Oppermann <andre@freebsd.org> Cc: freebsd-net@freebsd.org Subject: Re: vlan patch Message-ID: <20051019212057.GA48129@heff.fud.org.nz> In-Reply-To: <43562BBF.197CCF7E@freebsd.org> References: <20051019102559.GA45909@heff.fud.org.nz> <43562BBF.197CCF7E@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Wed, Oct 19, 2005 at 01:19:27PM +0200, Andre Oppermann wrote:
> Andrew Thompson wrote:
> >
> > Hi,
> >
> > It has always bugged me how the vlan code traverses the linked-list for
> > each incoming packet to find the right ifvlan, I have this patch which
> > attempts to fix this.
> >
>
> I like the concept. You may want to switch to direct indexing only
> when the number of vlan exceeds 10 or so.
>
Thanks for the feedback (and others too),
I have updated the patch to switch between array and linked-list
searching depending on the number of vlans.
cheers,
Andrew
--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="if_vlan12.diff"
Index: if_var.h
===================================================================
RCS file: /home/ncvs/src/sys/net/if_var.h,v
retrieving revision 1.103
diff -u -p -r1.103 if_var.h
--- if_var.h 1 Oct 2005 18:56:18 -0000 1.103
+++ if_var.h 19 Oct 2005 19:35:55 -0000
@@ -134,6 +134,7 @@ struct ifnet {
u_short if_index; /* numeric abbreviation for this if */
short if_timer; /* time 'til if_watchdog called */
u_short if_nvlans; /* number of active vlans */
+ /* correct location for *if_vlantags when changing ABI */
int if_flags; /* up/down, broadcast, etc. */
int if_capabilities; /* interface capabilities */
int if_capenable; /* enabled features */
@@ -180,6 +181,7 @@ struct ifnet {
struct task if_starttask; /* task for IFF_NEEDSGIANT */
struct task if_linktask; /* task for link change events */
struct mtx if_addr_mtx; /* mutex to protect address lists */
+ void *if_vlantags; /* array to hold active vlans */
};
typedef void if_init_f_t(void *);
Index: if_vlan.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_vlan.c,v
retrieving revision 1.88
diff -u -p -r1.88 if_vlan.c
--- if_vlan.c 3 Oct 2005 02:24:21 -0000 1.88
+++ if_vlan.c 19 Oct 2005 21:09:56 -0000
@@ -69,6 +69,7 @@
#endif
#define VLANNAME "vlan"
+#define VLAN_LIST_THRESHOLD 10
struct vlan_mc_entry {
struct ether_addr mc_addr;
@@ -133,7 +134,7 @@ static int vlan_setflag(struct ifnet *if
static int vlan_setflags(struct ifnet *ifp, int status);
static int vlan_setmulti(struct ifnet *ifp);
static int vlan_unconfig(struct ifnet *ifp);
-static int vlan_config(struct ifvlan *ifv, struct ifnet *p);
+static int vlan_config(struct ifvlan *ifv, struct ifnet *p, int tag);
static void vlan_link_state(struct ifnet *ifp, int link);
static struct ifnet *vlan_clone_match_ethertag(struct if_clone *,
@@ -406,7 +407,7 @@ vlan_clone_create(struct if_clone *ifc,
if (ethertag) {
VLAN_LOCK();
- error = vlan_config(ifv, p);
+ error = vlan_config(ifv, p, tag);
if (error != 0) {
/*
* Since we've partialy failed, we need to back
@@ -422,7 +423,6 @@ vlan_clone_create(struct if_clone *ifc,
return (error);
}
- ifv->ifv_tag = tag;
ifp->if_drv_flags |= IFF_DRV_RUNNING;
VLAN_UNLOCK();
@@ -568,10 +568,13 @@ static void
vlan_input(struct ifnet *ifp, struct mbuf *m)
{
struct ether_vlan_header *evl;
- struct ifvlan *ifv;
+ struct ifvlan *ifv = NULL;
+ struct ifvlan **ifv_tags;
struct m_tag *mtag;
u_int tag;
+ ifv_tags = ifp->if_vlantags;
+
if (m->m_flags & M_VLANTAG) {
/*
* Packet is tagged, but m contains a normal
@@ -620,9 +623,16 @@ vlan_input(struct ifnet *ifp, struct mbu
}
VLAN_LOCK();
- LIST_FOREACH(ifv, &ifv_list, ifv_list)
- if (ifp == ifv->ifv_p && tag == ifv->ifv_tag)
- break;
+ /* Look up the vlan in the parents tag array if allocated. O(1) */
+ if (ifp->if_vlantags != NULL)
+ ifv = ifv_tags[tag];
+ else {
+ /* Fall back to a linear search. O(n) */
+ LIST_FOREACH(ifv, &ifv_list, ifv_list)
+ if (ifp == ifv->ifv_p && tag == ifv->ifv_tag)
+ break;
+ }
+
if (ifv == NULL || (ifv->ifv_ifp->if_flags & IFF_UP) == 0) {
VLAN_UNLOCK();
@@ -658,18 +668,28 @@ vlan_input(struct ifnet *ifp, struct mbu
}
static int
-vlan_config(struct ifvlan *ifv, struct ifnet *p)
+vlan_config(struct ifvlan *ifv, struct ifnet *p, int tag)
{
struct ifaddr *ifa1, *ifa2;
struct ifnet *ifp;
struct sockaddr_dl *sdl1, *sdl2;
+ struct ifvlan *ifv2, **ifv_tags;
VLAN_LOCK_ASSERT();
+ ifv_tags = p->if_vlantags;
+
+ /* VID numbers 0x0 and 0xFFF are reserved */
+ if (tag == 0 || tag == EVL_VLID_MASK)
+ return (EINVAL);
if (p->if_type != IFT_ETHER)
return (EPROTONOSUPPORT);
if (ifv->ifv_p)
return (EBUSY);
+ /* Ensure unique ifnet + tag */
+ LIST_FOREACH(ifv2, &ifv_list, ifv_list)
+ if (p == ifv2->ifv_p && tag == ifv2->ifv_tag)
+ return (EBUSY);
ifv->ifv_encaplen = ETHER_VLAN_ENCAP_LEN;
ifv->ifv_mintu = ETHERMIN;
@@ -682,6 +702,27 @@ vlan_config(struct ifvlan *ifv, struct i
*/
p->if_nvlans++;
+ /* Allocate the parents tag array */
+ if (p->if_vlantags == NULL && p->if_nvlans > VLAN_LIST_THRESHOLD) {
+ p->if_vlantags = malloc(
+ (int)(EVL_VLID_MASK * sizeof(struct ifvlan *)),
+ M_VLAN, M_WAITOK | M_ZERO);
+ ifv_tags = p->if_vlantags;
+
+#ifdef DEBUG
+ printf("vlan: changing to array lookup for %s\n", p->if_xname);
+#endif
+
+ /* Fill it with existing vlans */
+ LIST_FOREACH(ifv2, &ifv_list, ifv_list)
+ if (ifv2->ifv_p == p)
+ ifv_tags[ifv2->ifv_tag] = ifv2;
+ }
+
+ ifv->ifv_tag = tag;
+ if (p->if_vlantags != NULL)
+ ifv_tags[tag] = ifv;
+
/*
* If the parent supports the VLAN_MTU capability,
* i.e. can Tx/Rx larger than ETHER_MAX_LEN frames,
@@ -763,6 +804,7 @@ vlan_unconfig(struct ifnet *ifp)
struct sockaddr_dl *sdl;
struct vlan_mc_entry *mc;
struct ifvlan *ifv;
+ struct ifvlan **ifv_tags;
struct ifnet *p;
int error;
@@ -798,7 +840,22 @@ vlan_unconfig(struct ifnet *ifp)
}
vlan_setflags(ifp, 0); /* clear special flags on parent */
+
+ ifv_tags = p->if_vlantags;
+ if (p->if_vlantags != NULL && ifv->ifv_tag > 0)
+ ifv_tags[ifv->ifv_tag] = NULL;
+
p->if_nvlans--;
+ /* Deallocate the parents tag array */
+ if (p->if_vlantags != NULL &&
+ p->if_nvlans <= VLAN_LIST_THRESHOLD) {
+ free(p->if_vlantags, M_VLAN);
+ p->if_vlantags = NULL;
+#ifdef DEBUG
+ printf("vlan: changing to list lookup for %s\n",
+ p->if_xname);
+#endif
+ }
}
/* Disconnect from parent. */
@@ -998,12 +1055,11 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
break;
}
VLAN_LOCK();
- error = vlan_config(ifv, p);
+ error = vlan_config(ifv, p, vlr.vlr_tag);
if (error) {
VLAN_UNLOCK();
break;
}
- ifv->ifv_tag = vlr.vlr_tag;
ifp->if_drv_flags |= IFF_DRV_RUNNING;
VLAN_UNLOCK();
--huq684BweRXVnRxX--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051019212057.GA48129>