On Wed, Apr 28, 2021 at 04:01:42PM +0200, Alexandr Nedvedicky wrote:
> On Wed, Apr 28, 2021 at 03:31:41PM +0200, Claudio Jeker wrote:
> > Another option is to assert on the condition. It is an error case
> > that should not exist.
> > 
> 
>     I think assert won't hurt here as it will enable us to learn something.

So let's commit my mq_delist() diff and after that commit the assert.
Then the latter can be easily tested and reverted if needed.

ok?

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);
 }

Reply via email to