On Wed, 6 Jan 2016, Mike Belopuhov wrote:
> There's still stuff to do, but it receives and transmits reliably
> (at least on modern Xen) so I'd like to get it in. Man page will
> follow.
I only had a quick glance at the code, but I have one comment about your
use of memory barriers. The membar_* macros are pure compiler barriers
when the openbsd kernel is compiled for UP. But since the host machine and
xen may use SMP even in this case, I suspect the that you need hardware
memory barriers even if MULTIPROCESSOR is not defined. This does not seem
relevant for x86 because you don't use membar_sync, but it may become
relevant for arm, which is also supported by xen.
I had the same problem in virtio and introduced the virtio_membar_* macros
for this purpose. Maybe they should be renamed to a more generic name and
you should use them, too?
> + if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(&ifp->if_snd))
> + return;
> +
> + prod = txr->txr_prod;
> + membar_consumer();
> +
> + for (;;) {
> + m = ifq_deq_begin(&ifp->if_snd);
> + if (m == NULL)
> + break;
> +
> + error = xnf_encap(sc, m, &prod);
> + if (error == ENOENT) {
> + /* transient */
> + ifq_deq_rollback(&ifp->if_snd, m);
> + ifq_set_oactive(&ifp->if_snd);
> + break;
> + } else if (error) {
> + /* the chain is too large */
> + ifq_deq_commit(&ifp->if_snd, m);
> + m_freem(m);
> + continue;
> + }
> + ifq_deq_commit(&ifp->if_snd, m);
> +
> +#if NBPFILTER > 0
> + if (ifp->if_bpf)
> + bpf_mtap_ether(ifp->if_bpf, m, BPF_DIRECTION_OUT);
> +#endif
> + pkts++;
> + }
> + if (pkts > 0) {
> + txr->txr_prod = prod;
> + xen_intr_signal(sc->sc_xih);
> + }
> +}
> +
> +int
> +xnf_encap(struct xnf_softc *sc, struct mbuf *m, uint32_t *prod)
> +{
> + struct ifnet *ifp = &sc->sc_ac.ac_if;
> + struct xnf_tx_ring *txr = sc->sc_tx_ring;
> + union xnf_tx_desc *txd;
> + bus_dmamap_t dmap;
> + int error, i, n = 0;
> +
> + if (((txr->txr_cons - *prod - 1) & (XNF_TX_DESC - 1)) < XNF_TX_FRAG) {
> + error = ENOENT;
> + goto errout;
> + }
> +
> + i = *prod & (XNF_TX_DESC - 1);
> + dmap = sc->sc_tx_dmap[i];
> +
> + error = bus_dmamap_load_mbuf(sc->sc_dmat, dmap, m, BUS_DMA_WRITE |
> + BUS_DMA_NOWAIT);
> + if (error == EFBIG) {
> + if (m_defrag(m, M_DONTWAIT) ||
> + bus_dmamap_load_mbuf(sc->sc_dmat, dmap, m, BUS_DMA_WRITE |
> + BUS_DMA_NOWAIT))
> + goto errout;
> + } else if (error)
> + goto errout;
> +
> + for (n = 0; n < dmap->dm_nsegs; n++, (*prod)++) {
> + i = *prod & (XNF_TX_DESC - 1);
> + if (sc->sc_tx_buf[i])
> + panic("%s: save vs spell: %d\n", ifp->if_xname, i);
> + txd = &txr->txr_desc[i];
> + if (n == 0) {
> + sc->sc_tx_buf[i] = m;
> + if (0 && m->m_pkthdr.csum_flags & M_IPV4_CSUM_OUT)
> + txd->txd_req.txq_flags = XNF_TXF_CSUM |
> + XNF_TXF_VALID;
> + txd->txd_req.txq_size = m->m_pkthdr.len;
> + } else
> + txd->txd_req.txq_size = dmap->dm_segs[n].ds_len;
> + if (n != dmap->dm_nsegs - 1)
> + txd->txd_req.txq_flags |= XNF_TXF_CHUNK;
> + txd->txd_req.txq_ref = dmap->dm_segs[n].ds_addr;
> + txd->txd_req.txq_offset = dmap->dm_segs[n].ds_offset;
> + }
> +
> + ifp->if_opackets++;
> + return (0);
> +
> + errout:
> + ifp->if_oerrors++;
> + return (error);
> +}
> +
> +void
> +xnf_intr(void *arg)
> +{
> + struct xnf_softc *sc = arg;
> + struct ifnet *ifp = &sc->sc_ac.ac_if;
> +
> + if (ifp->if_flags & IFF_RUNNING) {
> + xnf_rxeof(sc);
> + xnf_txeof(sc);
> + }
> +}
> +
> +int
> +xnf_txeof(struct xnf_softc *sc)
> +{
> + struct ifnet *ifp = &sc->sc_ac.ac_if;
> + struct xnf_tx_ring *txr = sc->sc_tx_ring;
> + union xnf_tx_desc *txd;
> + struct mbuf *m;
> + bus_dmamap_t dmap;
> + volatile uint32_t r;
> + uint32_t cons;
> + int i, id, pkts = 0;
> +
> + do {
> + for (cons = sc->sc_tx_cons; cons != txr->txr_cons; cons++) {
> + membar_consumer();
> + i = cons & (XNF_TX_DESC - 1);
> + txd = &txr->txr_desc[i];
> + id = txd->txd_rsp.txp_id;
> + memset(txd, 0, sizeof(*txd));
> + txd->txd_req.txq_id = id;
> + membar_producer();
> + if (sc->sc_tx_buf[i]) {
> + dmap = sc->sc_tx_dmap[i];
> + bus_dmamap_unload(sc->sc_dmat, dmap);
> + m = sc->sc_tx_buf[i];
> + sc->sc_tx_buf[i] = NULL;
> + m_freem(m);
> + }
> + pkts++;
> + }
> +
> + if (pkts > 0) {
> + sc->sc_tx_cons = cons;
> + membar_producer();
> + txr->txr_rsp_evt = cons + 1;
> + pkts = 0;
> + }
> +
> + r = txr->txr_cons - sc->sc_tx_cons;
> + membar_consumer();
> + } while (r > 0);
> +
> + if (ifq_is_oactive(&ifp->if_snd))
> + ifq_restart(&ifp->if_snd);
> +
> + return (0);
> +}
