On 21/03/15(Sat) 17:48, David Gwynne wrote:
>
> > On 19 Mar 2015, at 11:11 pm, Martin Pieuchot <[email protected]> wrote:
> >
> > When a host want to send packets to a destination whose Ethernet address
> > that has not been resolved yet, it puts such packet on a mbuf queue.
> >
> > Right now this queue, linked to the corresponding ARP data structure, is
> > hand rolled. I wrote the diff below during s2k15 to make use of the
> > mq_enqueue(9) API instead.
> >
> > I verified that the queue is correctly dropped when an infinite recursion
> > in in_arpinput() is triggered.
> >
> > Comments, Ok?
>
> the global count is annoying to read, but i get it.
>
> why mbuf_queues instead of mbuf_lists? just to get the drop on mq_enqueue?
Yep... That might be overkill since we do not really use the mutex. Do
you prefer the version below using a mbuf_list?
As a bonus this diff only call ml_init() if the pool allocation succeed.
Index: netinet/if_ether.c
===================================================================
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.148
diff -u -p -r1.148 if_ether.c
--- netinet/if_ether.c 14 Mar 2015 17:13:44 -0000 1.148
+++ netinet/if_ether.c 21 Mar 2015 13:59:55 -0000
@@ -69,7 +69,6 @@
#endif
#define SDL(s) ((struct sockaddr_dl *)s)
-#define SRP(s) ((struct sockaddr_inarp *)s)
/*
* ARP trailer negotiation. Trailer protocol is not IP specific,
@@ -77,6 +76,15 @@
*/
#define ETHERTYPE_IPTRAILERS ETHERTYPE_TRAIL
+struct llinfo_arp {
+ LIST_ENTRY(llinfo_arp) la_list;
+ struct rtentry *la_rt; /* backpointer to rtentry */
+ long la_asked; /* last time we QUERIED */
+ struct mbuf_list la_ml; /* packet hold queue */
+};
+#define LA_HOLD_QUEUE 10
+#define LA_HOLD_TOTAL 100
+
/* timer values */
int arpt_prune = (5*60*1); /* walk list every 5 minutes */
int arpt_keep = (20*60); /* once resolved, good for 20 more minutes */
@@ -227,6 +235,7 @@ arp_rtrequest(int req, struct rtentry *r
}
arp_inuse++;
arp_allocated++;
+ ml_init(&la->la_ml);
la->la_rt = rt;
rt->rt_flags |= RTF_LLINFO;
LIST_INSERT_HEAD(&llinfo_arp, la, la_list);
@@ -282,8 +291,7 @@ arp_rtrequest(int req, struct rtentry *r
LIST_REMOVE(la, la_list);
rt->rt_llinfo = 0;
rt->rt_flags &= ~RTF_LLINFO;
- while ((m = la->la_hold_head) != NULL) {
- la->la_hold_head = la->la_hold_head->m_nextpkt;
+ while ((m = ml_dequeue(&la->la_ml)) != NULL) {
la_hold_total--;
m_freem(m);
}
@@ -425,32 +433,19 @@ arpresolve(struct arpcom *ac, struct rte
* response yet. Insert mbuf in hold queue if below limit
* if above the limit free the queue without queuing the new packet.
*/
- if (la_hold_total < MAX_HOLD_TOTAL && la_hold_total < nmbclust / 64) {
- if (la->la_hold_count >= MAX_HOLD_QUEUE) {
- mh = la->la_hold_head;
- la->la_hold_head = la->la_hold_head->m_nextpkt;
- if (mh == la->la_hold_tail)
- la->la_hold_tail = NULL;
- la->la_hold_count--;
+ if (la_hold_total < LA_HOLD_TOTAL && la_hold_total < nmbclust / 64) {
+ if (ml_len(&la->la_ml) >= LA_HOLD_QUEUE) {
+ mh = ml_dequeue(&la->la_ml);
la_hold_total--;
m_freem(mh);
}
- if (la->la_hold_tail == NULL)
- la->la_hold_head = m;
- else
- la->la_hold_tail->m_nextpkt = m;
- la->la_hold_tail = m;
- la->la_hold_count++;
+ ml_enqueue(&la->la_ml, m);
la_hold_total++;
} else {
- while ((mh = la->la_hold_head) != NULL) {
- la->la_hold_head =
- la->la_hold_head->m_nextpkt;
+ while ((mh = ml_dequeue(&la->la_ml)) != NULL) {
la_hold_total--;
m_freem(mh);
}
- la->la_hold_tail = NULL;
- la->la_hold_count = 0;
m_freem(m);
}
@@ -483,14 +478,10 @@ arpresolve(struct arpcom *ac, struct rte
rt->rt_flags |= RTF_REJECT;
rt->rt_expire += arpt_down;
la->la_asked = 0;
- while ((mh = la->la_hold_head) != NULL) {
- la->la_hold_head =
- la->la_hold_head->m_nextpkt;
+ while ((mh = ml_dequeue(&la->la_ml)) != NULL) {
la_hold_total--;
m_freem(mh);
}
- la->la_hold_tail = NULL;
- la->la_hold_count = 0;
}
}
}
@@ -570,13 +561,14 @@ in_arpinput(struct mbuf *m)
struct sockaddr_dl *sdl;
struct sockaddr sa;
struct in_addr isaddr, itaddr, myaddr;
- struct mbuf *mh, *mt;
+ struct mbuf *mh;
u_int8_t *enaddr = NULL;
#if NCARP > 0
u_int8_t *ether_shost = NULL;
#endif
char addr[INET_ADDRSTRLEN];
int op, changed = 0;
+ unsigned int len;
ea = mtod(m, struct ether_arp *);
op = ntohs(ea->arp_op);
@@ -732,25 +724,19 @@ in_arpinput(struct mbuf *m)
if (la->la_asked || changed)
rt_sendmsg(rt, RTM_RESOLVE, rt->rt_ifp->if_rdomain);
la->la_asked = 0;
- while ((mh = la->la_hold_head) != NULL) {
- if ((la->la_hold_head = mh->m_nextpkt) == NULL)
- la->la_hold_tail = NULL;
- la->la_hold_count--;
+ while ((len = ml_len(&la->la_ml)) != 0) {
+ mh = ml_dequeue(&la->la_ml);
la_hold_total--;
- mt = la->la_hold_tail;
(*ac->ac_if.if_output)(&ac->ac_if, mh, rt_key(rt), rt);
- if (la->la_hold_tail == mh) {
+ if (ml_len(&la->la_ml) == len) {
/* mbuf is back in queue. Discard. */
- la->la_hold_tail = mt;
- if (la->la_hold_tail)
- la->la_hold_tail->m_nextpkt = NULL;
- else
- la->la_hold_head = NULL;
- la->la_hold_count--;
- la_hold_total--;
- m_freem(mh);
+ while ((mh = ml_dequeue(&la->la_ml)) != NULL) {
+ la_hold_total--;
+ m_freem(mh);
+ }
+ break;
}
}
}
@@ -1114,8 +1100,7 @@ db_print_llinfo(caddr_t li)
if (li == 0)
return;
la = (struct llinfo_arp *)li;
- db_printf(" la_rt=%p la_hold_head=%p, la_asked=0x%lx\n",
- la->la_rt, la->la_hold_head, la->la_asked);
+ db_printf(" la_rt=%p la_asked=0x%lx\n", la->la_rt, la->la_asked);
}
/*
Index: netinet/if_ether.h
===================================================================
RCS file: /cvs/src/sys/netinet/if_ether.h,v
retrieving revision 1.54
diff -u -p -r1.54 if_ether.h
--- netinet/if_ether.h 5 Dec 2014 15:50:04 -0000 1.54
+++ netinet/if_ether.h 19 Mar 2015 12:20:04 -0000
@@ -164,17 +164,6 @@ struct arpcom {
int ac_multirangecnt; /* number of mcast ranges */
};
-
-struct llinfo_arp {
- LIST_ENTRY(llinfo_arp) la_list;
- struct rtentry *la_rt;
- struct mbuf *la_hold_head; /* packet hold queue */
- struct mbuf *la_hold_tail;
- int la_hold_count; /* number of packets queued */
- long la_asked; /* last time we QUERIED for this addr */
-};
-#define MAX_HOLD_QUEUE 10
-#define MAX_HOLD_TOTAL 100
#endif
struct sockaddr_inarp {
