> On 19 Mar 2015, at 11:11 pm, Martin Pieuchot <m...@openbsd.org> 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?
>
> 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 19 Mar 2015 13:04:28 -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_queue la_mq; /* 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 */
> @@ -220,6 +228,7 @@ arp_rtrequest(int req, struct rtentry *r
> * add with a LL address.
> */
> la = pool_get(&arp_pool, PR_NOWAIT | PR_ZERO);
> + mq_init(&la->la_mq, LA_HOLD_QUEUE, IPL_NONE);
> rt->rt_llinfo = (caddr_t)la;
> if (la == NULL) {
> log(LOG_DEBUG, "%s: malloc failed\n", __func__);
> @@ -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 = mq_dequeue(&la->la_mq)) != NULL) {
> la_hold_total--;
> m_freem(m);
> }
> @@ -425,32 +433,14 @@ 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--;
> - 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++;
> - la_hold_total++;
> + if (la_hold_total < LA_HOLD_TOTAL && la_hold_total < nmbclust / 64) {
> + if (mq_enqueue(&la->la_mq, m) == 0)
> + la_hold_total++;
> } else {
> - while ((mh = la->la_hold_head) != NULL) {
> - la->la_hold_head =
> - la->la_hold_head->m_nextpkt;
> + while ((mh = mq_dequeue(&la->la_mq)) != NULL) {
> la_hold_total--;
> m_freem(mh);
> }
> - la->la_hold_tail = NULL;
> - la->la_hold_count = 0;
> m_freem(m);
> }
>
> @@ -483,14 +473,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 = mq_dequeue(&la->la_mq)) != NULL) {
> la_hold_total--;
> m_freem(mh);
> }
> - la->la_hold_tail = NULL;
> - la->la_hold_count = 0;
> }
> }
> }
> @@ -570,13 +556,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 +719,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 = mq_len(&la->la_mq)) != 0) {
> + mh = mq_dequeue(&la->la_mq);
> 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 (mq_len(&la->la_mq) == 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 = mq_dequeue(&la->la_mq)) != NULL) {
> + la_hold_total--;
> + m_freem(mh);
> + }
> + break;
> }
> }
> }
> @@ -1114,8 +1095,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);
> }
>
> /*
