I just whipped up this against
5.3-STABLE #1: Wed Dec 22 17:11:02 PST 2004
Would someone who knows a bit more about this than myself give it a
quick lookover and see if it appears sane? I'm mostly wondering about
the splimp() and splx() and whether it's required or excessive due to
the mtx_lock/unlock in the VLAN_LOCK/UNLOCK macros.
Due to a lack of equipment it's difficult for me to run a seperate
test environment, so any sort of review would be appreciated.
--- sys/net/if_vlan.c.orig Wed Jan 5 12:25:19 2005
+++ sys/net/if_vlan.c Wed Jan 5 12:53:45 2005
@@ -379,7 +379,10 @@
ifp->if_init = vlan_ifinit;
ifp->if_start = vlan_start;
ifp->if_ioctl = vlan_ioctl;
- ifp->if_snd.ifq_maxlen = ifqmaxlen;
+ IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
+ ifp->if_snd.ifq_drv_maxlen = 0;
+ IFQ_SET_READY(&ifp->if_snd);
+
ether_ifattach(ifp, ifv->ifv_ac.ac_enaddr);
/* Now undo some of the damage... */
ifp->if_baudrate = 0;
@@ -423,11 +426,15 @@
{
int unit;
struct ifvlan *ifv = ifp->if_softc;
+ int s;
unit = ifp->if_dunit;
VLAN_LOCK();
LIST_REMOVE(ifv, ifv_list);
+ s = splimp();
+ IFQ_PURGE(&ifp->if_snd);
+ splx(s);
vlan_unconfig(ifp);
VLAN_UNLOCK();
@@ -458,12 +465,22 @@
struct mbuf *m;
int error;
+ if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
+ IFQ_LOCK(&ifp->if_snd);
+ IFQ_POLL_NOLOCK(&ifp->if_snd, m);
+ if (m == NULL ) {
+ IFQ_UNLOCK(&ifp->if_snd);
+ return;
+ }
+ IFQ_UNLOCK(&ifp->if_snd);
+ }
+
ifv = ifp->if_softc;
p = ifv->ifv_p;
ifp->if_flags |= IFF_OACTIVE;
for (;;) {
- IF_DEQUEUE(&ifp->if_snd, m);
+ IFQ_DEQUEUE(&ifp->if_snd, m);
if (m == 0)
break;
BPF_MTAP(ifp, m);
On Wed, Jan 05, 2005 at 12:32:55PM -0800, Jon Simola wrote:> I just whipped up this against > 5.3-STABLE #1: Wed Dec 22 17:11:02 PST 2004 > > Would someone who knows a bit more about this than myself give it a > quick lookover and see if it appears sane? I'm mostly wondering about > the splimp() and splx() and whether it's required or excessive due to > the mtx_lock/unlock in the VLAN_LOCK/UNLOCK macros. > > Due to a lack of equipment it's difficult for me to run a seperate > test environment, so any sort of review would be appreciated.ALTQ makes no sense of virtual interfaces. ALTQ works by providing fine-grained control of the dequeueing of packets on to the wire. It's too early to do this when you're still in the virtual interface. You can tag packets appropiratly at this point, but the actual ALTQ queue needs to be on a physical interface. See this thread on adding ALTQ to gif(4): http://www.mail-archive.com/freebsd-net@freebsd.org/msg13875.html FYI, spl*() funtions are all no-ops now. We just have them around to remind us that we need to lock certain functions and to document what was protected before. -- Brooks
0n Wed, Jan 05, 2005 at 12:51:56PM -0800, Brooks Davis wrote:
>FYI, spl*() funtions are all no-ops now. We just have them around to
>remind us that we need to lock certain functions and to document what
>was protected before.
What is meant by "no-ops" ?
- aW
On Thu, Jan 06, 2005 at 05:00:32PM +1030, Wilkinson, Alex wrote:> 0n Wed, Jan 05, 2005 at 12:51:56PM -0800, Brooks Davis wrote: > > >FYI, spl*() funtions are all no-ops now. We just have them around to > >remind us that we need to lock certain functions and to document what > >was protected before. > > What is meant by "no-ops" ?They do nothing. -- Brooks -- Any statement of the form "X is the one, true Y" is FALSE. PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: not available Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20050105/b774c732/attachment.bin