On 13/12/17(Wed) 09:54, David Gwynne wrote: > im still looking at vlan performance problems, as discussed by mpi@ > at http://www.grenadille.net/post/2017/02/13/What-happened-to-my-vlan. > > recently it occurred to me that we're making an implicit assumption > that having the chip handle the injection of vlan tags has zero > cost, and that all the loss in performance is purely a software > problem. to test this assumption i knocked up the diff below to > disable hw vlan tagging in ix(4), which was used in the tests mpi > and hrvoje did. > > hrvoje tested this diff for me and noted a 10% improvement in pps > when forwarding between vlan interfaces on ix(4). to quote hrvoje: > > without diff > send - receive > vlan - vlan = 830Kpps > > with diff > send - receive > vlan - vlan = 995Kpps > > my conclusion is that assumption that nics are fast at offloads is > wrong. therefore id like to put this in. unfortunately 10% doesnt > account for the entire loss in forwarding over vlan, but it does > help a bit. > > would anyone else like to test? or ok it?
I don't have hardware to test but I'd like to add that in bridge(4) scenario hardware tagging also decrease performance. >From my point of view removing this per-chip option makes the stack simpler, so I'm all for it. However I'd like to hear more test reports on different ix(4) models. > Index: if_ix.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/if_ix.c,v > retrieving revision 1.152 > diff -u -p -r1.152 if_ix.c > --- if_ix.c 22 Jun 2017 02:44:37 -0000 1.152 > +++ if_ix.c 12 Dec 2017 23:39:41 -0000 > @@ -154,8 +154,6 @@ void ixgbe_set_ivar(struct ix_softc *, u > void ixgbe_configure_ivars(struct ix_softc *); > uint8_t *ixgbe_mc_array_itr(struct ixgbe_hw *, uint8_t **, uint32_t *); > > -void ixgbe_setup_vlan_hw_support(struct ix_softc *); > - > /* Support for pluggable optic modules */ > void ixgbe_setup_optics(struct ix_softc *); > void ixgbe_handle_mod(struct ix_softc *); > @@ -702,9 +700,6 @@ ixgbe_init(void *arg) > IXGBE_WRITE_REG(&sc->hw, IXGBE_RDT(i), rxr->last_desc_filled); > } > > - /* Set up VLAN support and filter */ > - ixgbe_setup_vlan_hw_support(sc); > - > /* Enable Receive engine */ > rxctrl = IXGBE_READ_REG(&sc->hw, IXGBE_RXCTRL); > if (sc->hw.mac.type == ixgbe_mac_82598EB) > @@ -1621,10 +1616,6 @@ ixgbe_setup_interface(struct ix_softc *s > > ifp->if_capabilities = IFCAP_VLAN_MTU; > > -#if NVLAN > 0 > - ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING; > -#endif > - > #ifdef IX_CSUM_OFFLOAD > ifp->if_capabilities |= IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4; > #endif > @@ -2922,12 +2913,6 @@ ixgbe_rxeof(struct ix_queue *que) > /* first desc of a non-ps chain */ > sendmp = mp; > sendmp->m_pkthdr.len = mp->m_len; > -#if NVLAN > 0 > - if (staterr & IXGBE_RXD_STAT_VP) { > - sendmp->m_pkthdr.ether_vtag = vtag; > - sendmp->m_flags |= M_VLANTAG; > - } > -#endif > } > > /* Pass the head pointer on */ > @@ -2989,44 +2974,6 @@ ixgbe_rx_checksum(uint32_t staterr, stru > if (!(errors & IXGBE_RXD_ERR_TCPE)) > mp->m_pkthdr.csum_flags |= > M_TCP_CSUM_IN_OK | M_UDP_CSUM_IN_OK; > - } > -} > - > -void > -ixgbe_setup_vlan_hw_support(struct ix_softc *sc) > -{ > - uint32_t ctrl; > - int i; > - > - /* > - * A soft reset zero's out the VFTA, so > - * we need to repopulate it now. > - */ > - for (i = 0; i < IXGBE_VFTA_SIZE; i++) { > - if (sc->shadow_vfta[i] != 0) > - IXGBE_WRITE_REG(&sc->hw, IXGBE_VFTA(i), > - sc->shadow_vfta[i]); > - } > - > - ctrl = IXGBE_READ_REG(&sc->hw, IXGBE_VLNCTRL); > -#if 0 > - /* Enable the Filter Table if enabled */ > - if (ifp->if_capenable & IFCAP_VLAN_HWFILTER) { > - ctrl &= ~IXGBE_VLNCTRL_CFIEN; > - ctrl |= IXGBE_VLNCTRL_VFE; > - } > -#endif > - if (sc->hw.mac.type == ixgbe_mac_82598EB) > - ctrl |= IXGBE_VLNCTRL_VME; > - IXGBE_WRITE_REG(&sc->hw, IXGBE_VLNCTRL, ctrl); > - > - /* On 82599 the VLAN enable is per/queue in RXDCTL */ > - if (sc->hw.mac.type != ixgbe_mac_82598EB) { > - for (i = 0; i < sc->num_queues; i++) { > - ctrl = IXGBE_READ_REG(&sc->hw, IXGBE_RXDCTL(i)); > - ctrl |= IXGBE_RXDCTL_VME; > - IXGBE_WRITE_REG(&sc->hw, IXGBE_RXDCTL(i), ctrl); > - } > } > } > >