On Wed, Apr 28, 2021 at 11:13:05PM +0300, Vitaliy Makkoveev wrote: > Also we have two cases where `la_mq??? is not empty: > 1. This thread put it back while performed ifp->if_output > 2. Concurrent thread put it???s own packet because ARP resolution failed. > > So I doubt ???XXXSMP??? and following ???mbuf is back in queue..??? comments > are > correct as is. May be it???s better to combine them?
I want to delete this code in next commit and want to replace it with an assertion. Or do you object this idea? Which comment should I insert here to delete in next commit? This diff is about replacing mq_dequeue with mq_delist as sashan@ suggested. The comments are unrelated and are just moved around. bluhm > > Index: netinet/if_ether.c > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/if_ether.c,v > > retrieving revision 1.247 > > diff -u -p -r1.247 if_ether.c > > --- netinet/if_ether.c 28 Apr 2021 10:33:34 -0000 1.247 > > +++ netinet/if_ether.c 28 Apr 2021 19:35:19 -0000 > > @@ -611,7 +611,9 @@ arpcache(struct ifnet *ifp, struct ether > > struct in_addr *spa = (struct in_addr *)ea->arp_spa; > > char addr[INET_ADDRSTRLEN]; > > struct ifnet *rifp; > > + struct mbuf_list ml; > > struct mbuf *m; > > + unsigned int len; > > int changed = 0; > > > > KERNEL_ASSERT_LOCKED(); > > @@ -683,21 +685,17 @@ arpcache(struct ifnet *ifp, struct ether > > > > la->la_asked = 0; > > la->la_refreshed = 0; > > - while ((m = mq_dequeue(&la->la_mq)) != NULL) { > > - unsigned int len; > > - > > - atomic_dec_int(&la_hold_total); > > - len = mq_len(&la->la_mq); > > - > > + mq_delist(&la->la_mq, &ml); > > + len = ml_len(&ml); > > + while ((m = ml_dequeue(&ml)) != NULL) { > > ifp->if_output(ifp, m, rt_key(rt), rt); > > - > > - /* XXXSMP we discard if other CPU enqueues */ > > - if (mq_len(&la->la_mq) > len) { > > - /* mbuf is back in queue. Discard. */ > > - atomic_sub_int(&la_hold_total, mq_purge(&la->la_mq)); > > - break; > > - } > > } > > + /* XXXSMP we discard if other CPU enqueues */ > > + if (mq_len(&la->la_mq) > 0) { > > + /* mbuf is back in queue. Discard. */ > > + atomic_sub_int(&la_hold_total, len + mq_purge(&la->la_mq)); > > + } else > > + atomic_sub_int(&la_hold_total, len); > > > > return (0); > > } > > >