On Tue, Dec 28, 2021 at 04:10:52PM +0100, Alessandro De Laurenzis wrote:
> Ciao David,
> 
> > msk is the first port added to the trunk? ie, it's the preferred port? if 
> > you run tcpdump on msk or watch systat if, do you see packets on msk?
> 
> The network config is pretty standard; an Ethernet port (msk0), a wifi one
> (iwn0), trunk0 with failover (using msk0 as "preferred" port):
> 
> > $ cat /etc/hostname.trunk0 trunkproto failover
> > trunkport msk0
> > trunkport iwn0
> > autoconf
> > up
> 
> Bear with me, tcpdump is a kind of stranger world for me...
> 
> Enclosed please find the output files from the following commands:
> 
> > $ doas tcpdump -i trunk0 -c 50 -w trunk0.dump
> > $ doas tcpdump -i msk0 -c 50 -w msk0.dump
> > $ doas tcpdump -i iwn0 -c 50 -w iwn0.dump
> 
> I see some "broadcast" packages on both trunk0 and msk0 (trunk0 didn't
> receive the inet address from the DHCP server, of course); nothing as
> expected on iwn0.
> 
> Hope this answers to your question...

it was hard to see the trees because of the forest :(

im back in the office today, so i dug out a box with msk and was able to
reproduce the problem.

it looks like the hardware "remembers" stuff between when an msk port is
taken down and goes up again, like what trunk does to an interface when
it's added as a port. i think the problem would occur if you just took
msk down and up again during normal operation too, but that seems to be
a rare event...

the diff that borked it did so because of how it accounted for free
space on the ring, and was otherwise lucky.

can you try this diff?

Index: if_msk.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_msk.c,v
retrieving revision 1.136
diff -u -p -r1.136 if_msk.c
--- if_msk.c    12 Dec 2020 11:48:53 -0000      1.136
+++ if_msk.c    4 Jan 2022 08:06:11 -0000
@@ -135,7 +135,7 @@ int msk_intr(void *);
 void msk_intr_yukon(struct sk_if_softc *);
 static inline int msk_rxvalid(struct sk_softc *, u_int32_t, u_int32_t);
 void msk_rxeof(struct sk_if_softc *, struct mbuf_list *, uint16_t, uint32_t);
-void msk_txeof(struct sk_if_softc *);
+void msk_txeof(struct sk_if_softc *, unsigned int);
 static unsigned int msk_encap(struct sk_if_softc *, struct mbuf *, uint32_t);
 void msk_start(struct ifnet *);
 int msk_ioctl(struct ifnet *, u_long, caddr_t);
@@ -1561,7 +1561,7 @@ msk_watchdog(struct ifnet *ifp)
         * Reclaim first as there is a possibility of losing Tx completion
         * interrupts.
         */
-       msk_txeof(sc_if);
+       //msk_txeof(sc_if, sc_if->sk_cdata.sk_tx_prod);
        if (sc_if->sk_cdata.sk_tx_prod != sc_if->sk_cdata.sk_tx_cons) {
                printf("%s: watchdog timeout\n", sc_if->sk_dev.dv_xname);
 
@@ -1639,26 +1639,19 @@ msk_rxeof(struct sk_if_softc *sc_if, str
 }
 
 void
-msk_txeof(struct sk_if_softc *sc_if)
+msk_txeof(struct sk_if_softc *sc_if, unsigned int prod)
 {
        struct ifnet            *ifp = &sc_if->arpcom.ac_if;
        struct sk_softc         *sc = sc_if->sk_softc;
-       uint32_t                prod, cons;
+       uint32_t                cons;
        struct mbuf             *m;
        bus_dmamap_t            map;
-       bus_size_t              reg;
-
-       if (sc_if->sk_port == SK_PORT_A)
-               reg = SK_STAT_BMU_TXA1_RIDX;
-       else
-               reg = SK_STAT_BMU_TXA2_RIDX;
 
        /*
         * Go through our tx ring and free mbufs for those
         * frames that have been sent.
         */
        cons = sc_if->sk_cdata.sk_tx_cons;
-       prod = sk_win_read_2(sc, reg);
 
        if (cons == prod)
                return;
@@ -1770,7 +1763,7 @@ msk_intr(void *xsc)
                                };
        struct ifnet            *ifp0 = NULL, *ifp1 = NULL;
        int                     claimed = 0;
-       u_int32_t               status;
+       u_int32_t               status, sk_status;
        struct msk_status_desc  *cur_st;
 
        status = CSR_READ_4(sc, SK_Y2_ISSR2);
@@ -1812,10 +1805,19 @@ msk_intr(void *xsc)
                            lemtoh32(&cur_st->sk_status));
                        break;
                case SK_Y2_STOPC_TXSTAT:
+                       sk_status = lemtoh32(&cur_st->sk_status);
                        if (sc_if0)
-                               msk_txeof(sc_if0);
-                       if (sc_if1)
-                               msk_txeof(sc_if1);
+                               msk_txeof(sc_if0, sk_status & 0xfff);
+                       if (sc_if1) {
+                               /*
+                                * this would be easier as a 64bit
+                                * load of the whole status descriptor,
+                                * a shift, and a mask.
+                                */
+                               unsigned int cons = (sk_status >> 24) & 0xff;
+                               cons |= (lemtoh16(&cur_st->sk_len) & 0xf) << 8;
+                               msk_txeof(sc_if1, cons);
+                       }
                        break;
                default:
                        printf("opcode=0x%x\n", cur_st->sk_opcode);
@@ -2065,8 +2067,8 @@ msk_init(void *xsc_if)
 
        SK_IF_WRITE_2(sc_if, 0, SK_RXQ1_Y2_PREF_PUTIDX,
            sc_if->sk_cdata.sk_rx_prod);
-       SK_IF_WRITE_2(sc_if, 1, SK_TXQA1_Y2_PREF_PUTIDX,
-           sc_if->sk_cdata.sk_tx_prod);
+       SK_IF_WRITE_2(sc_if, 1, SK_TXQA1_Y2_PREF_PUTIDX, 0);
+           //sc_if->sk_cdata.sk_tx_prod);
 
        /* Configure interrupt handling */
        if (sc_if->sk_port == SK_PORT_A)

Reply via email to