Date: Thu, 18 Aug 2016 06:29:07 +0000 (UTC) From: Pyun YongHyeon <yongari@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r304332 - head/sys/dev/usb/net Message-ID: <201608180629.u7I6T7ik026391@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: yongari Date: Thu Aug 18 06:29:07 2016 New Revision: 304332 URL: https://svnweb.freebsd.org/changeset/base/304332 Log: Introduce axge_rxfilter() which configures RX filtering and replace axge_setmulti()/axge_setpromisc() with axge_rxfilter(). Multicast filter programming and promiscuous mode requires access to a common RX configuration register so there is no need to use separate functions with added complexity. axge_rxfilter() does not read back AXGE_RCR register since accessing a register in USB is too slow and we already have all knowledge of required configuration. Rebuilding RX filter configuration is simpler and faster than manipulating every bits after reading back the register. Note, axge_rxfilter() does not set RCR_IPE(IP header alignment on 32bit boundary) to disable extra padding bytes insertion. The extra padding wastes ethernet to USB host bandwidth as well as complicating RX handling logic. Current USB framework requires copying RX frames to mbufs so there is no need to worry about alignment. Previously axge_rx_frame() performed wrong bound check due to the extra padding and it was broken when RX checksum offloading is disabled. See added comment in axge_rx_frame () for actual RX packet layout. In axge_init(), disable WOL. It's meaningless to enable WOL in normal operation. In axge_rxeof(), use properly sized mbuf rather than blindly allocating a mbuf cluster. Use RX H/W checksum offloading only when administrator requested RX checksum offloading. Previously it always used RX H/W checksum offloading result regardless of RX checksum offloading state. Separate L4 checksum offloading validation from L3 one and properly set required offloading bits for each layer. This is to fix setting L4 checksum offloading bits for L3 packets. There are still lots of RX errors(probably RX FIFO overflows) under moderate load. Users are strongly recommended to enable ethernet flow control. Reviewed by: kevlo (initial version), hselasky Modified: head/sys/dev/usb/net/if_axge.c head/sys/dev/usb/net/if_axgereg.h Modified: head/sys/dev/usb/net/if_axge.c ============================================================================== --- head/sys/dev/usb/net/if_axge.c Thu Aug 18 06:03:55 2016 (r304331) +++ head/sys/dev/usb/net/if_axge.c Thu Aug 18 06:29:07 2016 (r304332) @@ -104,8 +104,7 @@ static uether_fn_t axge_init; static uether_fn_t axge_stop; static uether_fn_t axge_start; static uether_fn_t axge_tick; -static uether_fn_t axge_setmulti; -static uether_fn_t axge_setpromisc; +static uether_fn_t axge_rxfilter; static int axge_read_mem(struct axge_softc *, uint8_t, uint16_t, uint16_t, void *, int); @@ -200,8 +199,8 @@ static const struct usb_ether_methods ax .ue_init = axge_init, .ue_stop = axge_stop, .ue_tick = axge_tick, - .ue_setmulti = axge_setmulti, - .ue_setpromisc = axge_setpromisc, + .ue_setmulti = axge_rxfilter, + .ue_setpromisc = axge_rxfilter, .ue_mii_upd = axge_ifmedia_upd, .ue_mii_sts = axge_ifmedia_sts, }; @@ -727,7 +726,7 @@ axge_tick(struct usb_ether *ue) } static void -axge_setmulti(struct usb_ether *ue) +axge_rxfilter(struct usb_ether *ue) { struct axge_softc *sc; struct ifnet *ifp; @@ -741,14 +740,26 @@ axge_setmulti(struct usb_ether *ue) h = 0; AXGE_LOCK_ASSERT(sc, MA_OWNED); - rxmode = axge_read_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_RCR); + /* + * Configure RX settings. + * Don't set RCR_IPE(IP header alignment on 32bit boundary) to disable + * inserting extra padding bytes. This wastes ethernet to USB host + * bandwidth as well as complicating RX handling logic. Current USB + * framework requires copying RX frames to mbufs so there is no need + * to worry about alignment. + */ + rxmode = RCR_DROP_CRCERR | RCR_START; + if (ifp->if_flags & IFF_BROADCAST) + rxmode |= RCR_ACPT_BCAST; if (ifp->if_flags & (IFF_ALLMULTI | IFF_PROMISC)) { + if (ifp->if_flags & IFF_PROMISC) + rxmode |= RCR_PROMISC; rxmode |= RCR_ACPT_ALL_MCAST; axge_write_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_RCR, rxmode); return; } - rxmode &= ~RCR_ACPT_ALL_MCAST; + rxmode |= RCR_ACPT_MCAST; if_maddr_rlock(ifp); TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { if (ifma->ifma_addr->sa_family != AF_LINK) @@ -764,26 +775,6 @@ axge_setmulti(struct usb_ether *ue) } static void -axge_setpromisc(struct usb_ether *ue) -{ - struct axge_softc *sc; - struct ifnet *ifp; - uint16_t rxmode; - - sc = uether_getsc(ue); - ifp = uether_getifp(ue); - rxmode = axge_read_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_RCR); - - if (ifp->if_flags & IFF_PROMISC) - rxmode |= RCR_PROMISC; - else - rxmode &= ~RCR_PROMISC; - - axge_write_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_RCR, rxmode); - axge_setmulti(ue); -} - -static void axge_start(struct usb_ether *ue) { struct axge_softc *sc; @@ -801,7 +792,6 @@ axge_init(struct usb_ether *ue) { struct axge_softc *sc; struct ifnet *ifp; - uint16_t rxmode; sc = uether_getsc(ue); ifp = uether_getifp(ue); @@ -827,25 +817,22 @@ axge_init(struct usb_ether *ue) /* Configure TX/RX checksum offloading. */ axge_csum_cfg(ue); - /* Configure RX settings. */ - rxmode = (RCR_ACPT_MCAST | RCR_START | RCR_DROP_CRCERR); - if ((ifp->if_capenable & IFCAP_RXCSUM) != 0) - rxmode |= RCR_IPE; - - /* If we want promiscuous mode, set the allframes bit. */ - if (ifp->if_flags & IFF_PROMISC) - rxmode |= RCR_PROMISC; - - if (ifp->if_flags & IFF_BROADCAST) - rxmode |= RCR_ACPT_BCAST; - - axge_write_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_RCR, rxmode); + /* Configure RX filters. */ + axge_rxfilter(ue); - axge_write_cmd_1(sc, AXGE_ACCESS_MAC, AXGE_MMSR, - MMSR_PME_TYPE | MMSR_PME_POL | MMSR_RWMP); + /* + * XXX + * Controller supports wakeup on link change detection, + * magic packet and wakeup frame recpetion. But it seems + * there is no framework for USB ethernet suspend/wakeup. + * Disable all wakeup functions. + */ + axge_write_cmd_1(sc, AXGE_ACCESS_MAC, AXGE_MMSR, 0); + (void)axge_read_cmd_1(sc, AXGE_ACCESS_MAC, AXGE_MMSR); - /* Load the multicast filter. */ - axge_setmulti(ue); + /* Configure default medium type. */ + axge_write_cmd_2(sc, AXGE_ACCESS_MAC, 2, AXGE_MSR, MSR_GM | MSR_FD | + MSR_RFC | MSR_TFC | MSR_RE); usbd_xfer_set_stall(sc->sc_xfer[AXGE_BULK_DT_WR]); @@ -921,12 +908,12 @@ axge_ioctl(struct ifnet *ifp, u_long cmd static void axge_rx_frame(struct usb_ether *ue, struct usb_page_cache *pc, int actlen) { - uint32_t pos; - uint32_t pkt_cnt; + struct axge_frame_rxhdr pkt_hdr; uint32_t rxhdr; - uint32_t pkt_hdr; + uint32_t pos; + uint32_t pkt_cnt, pkt_end; uint32_t hdr_off; - uint32_t pktlen; + uint32_t pktlen; /* verify we have enough data */ if (actlen < (int)sizeof(rxhdr)) @@ -937,41 +924,47 @@ axge_rx_frame(struct usb_ether *ue, stru usbd_copy_out(pc, actlen - sizeof(rxhdr), &rxhdr, sizeof(rxhdr)); rxhdr = le32toh(rxhdr); - pkt_cnt = (uint16_t)rxhdr; - hdr_off = (uint16_t)(rxhdr >> 16); + pkt_cnt = rxhdr & 0xFFFF; + hdr_off = pkt_end = (rxhdr >> 16) & 0xFFFF; + /* + * <----------------------- actlen ------------------------> + * [frame #0]...[frame #N][pkt_hdr #0]...[pkt_hdr #N][rxhdr] + * Each RX frame would be aligned on 8 bytes boundary. If + * RCR_IPE bit is set in AXGE_RCR register, there would be 2 + * padding bytes and 6 dummy bytes(as the padding also should + * be aligned on 8 bytes boundary) for each RX frame to align + * IP header on 32bits boundary. Driver don't set RCR_IPE bit + * of AXGE_RCR register, so there should be no padding bytes + * which simplifies RX logic a lot. + */ while (pkt_cnt--) { /* verify the header offset */ if ((int)(hdr_off + sizeof(pkt_hdr)) > actlen) { DPRINTF("End of packet headers\n"); break; } - if ((int)pos >= actlen) { + usbd_copy_out(pc, hdr_off, &pkt_hdr, sizeof(pkt_hdr)); + pkt_hdr.status = le32toh(pkt_hdr.status); + pktlen = AXGE_RXBYTES(pkt_hdr.status); + if (pos + pktlen > pkt_end) { DPRINTF("Data position reached end\n"); break; } - usbd_copy_out(pc, hdr_off, &pkt_hdr, sizeof(pkt_hdr)); - pkt_hdr = le32toh(pkt_hdr); - pktlen = (pkt_hdr >> 16) & 0x1fff; - if (pkt_hdr & (AXGE_RXHDR_CRC_ERR | AXGE_RXHDR_DROP_ERR)) { + if (AXGE_RX_ERR(pkt_hdr.status) != 0) { DPRINTF("Dropped a packet\n"); if_inc_counter(ue->ue_ifp, IFCOUNTER_IERRORS, 1); - } - if (pktlen >= 6 && (int)(pos + pktlen) <= actlen) { - axge_rxeof(ue, pc, pos + 2, pktlen - 6, pkt_hdr); - } else { - DPRINTF("Invalid packet pos=%d len=%d\n", - (int)pos, (int)pktlen); - } + } else + axge_rxeof(ue, pc, pos, pktlen, pkt_hdr.status); pos += (pktlen + 7) & ~7; hdr_off += sizeof(pkt_hdr); } } static void -axge_rxeof(struct usb_ether *ue, struct usb_page_cache *pc, - unsigned int offset, unsigned int len, uint32_t pkt_hdr) +axge_rxeof(struct usb_ether *ue, struct usb_page_cache *pc, unsigned int offset, + unsigned int len, uint32_t status) { struct ifnet *ifp; struct mbuf *m; @@ -982,29 +975,34 @@ axge_rxeof(struct usb_ether *ue, struct return; } - m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR); + if (len > MHLEN - ETHER_ALIGN) + m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR); + else + m = m_gethdr(M_NOWAIT, MT_DATA); if (m == NULL) { if_inc_counter(ifp, IFCOUNTER_IQDROPS, 1); return; } m->m_pkthdr.rcvif = ifp; - m->m_len = m->m_pkthdr.len = len + ETHER_ALIGN; - m_adj(m, ETHER_ALIGN); + m->m_len = m->m_pkthdr.len = len; + m->m_data += ETHER_ALIGN; usbd_copy_out(pc, offset, mtod(m, uint8_t *), len); - if_inc_counter(ifp, IFCOUNTER_IPACKETS, 1); - - if ((pkt_hdr & (AXGE_RXHDR_L4CSUM_ERR | AXGE_RXHDR_L3CSUM_ERR)) == 0) { - if ((pkt_hdr & AXGE_RXHDR_L4_TYPE_MASK) == - AXGE_RXHDR_L4_TYPE_TCP || - (pkt_hdr & AXGE_RXHDR_L4_TYPE_MASK) == - AXGE_RXHDR_L4_TYPE_UDP) { + if ((ifp->if_capenable & IFCAP_RXCSUM) != 0) { + if ((status & AXGE_RX_L3_CSUM_ERR) == 0 && + (status & AXGE_RX_L3_TYPE_MASK) == AXGE_RX_L3_TYPE_IPV4) + m->m_pkthdr.csum_flags |= CSUM_IP_CHECKED | + CSUM_IP_VALID; + if ((status & AXGE_RX_L4_CSUM_ERR) == 0 && + ((status & AXGE_RX_L4_TYPE_MASK) == AXGE_RX_L4_TYPE_UDP || + (status & AXGE_RX_L4_TYPE_MASK) == AXGE_RX_L4_TYPE_TCP)) { m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | - CSUM_PSEUDO_HDR | CSUM_IP_CHECKED | CSUM_IP_VALID; + CSUM_PSEUDO_HDR; m->m_pkthdr.csum_data = 0xffff; } } + if_inc_counter(ifp, IFCOUNTER_IPACKETS, 1); _IF_ENQUEUE(&ue->ue_rxq, m); } Modified: head/sys/dev/usb/net/if_axgereg.h ============================================================================== --- head/sys/dev/usb/net/if_axgereg.h Thu Aug 18 06:03:55 2016 (r304331) +++ head/sys/dev/usb/net/if_axgereg.h Thu Aug 18 06:29:07 2016 (r304332) @@ -142,14 +142,6 @@ #define AXGE_CONFIG_IDX 0 /* config number 1 */ #define AXGE_IFACE_IDX 0 -#define AXGE_RXHDR_L4_TYPE_MASK 0x1c -#define AXGE_RXHDR_L4CSUM_ERR 1 -#define AXGE_RXHDR_L3CSUM_ERR 2 -#define AXGE_RXHDR_L4_TYPE_UDP 4 -#define AXGE_RXHDR_L4_TYPE_TCP 16 -#define AXGE_RXHDR_CRC_ERR 0x20000000 -#define AXGE_RXHDR_DROP_ERR 0x80000000 - #define GET_MII(sc) uether_getmii(&(sc)->sc_ue) /* The interrupt endpoint is currently unused by the ASIX part. */ @@ -181,6 +173,33 @@ struct axge_frame_txhdr { #define AXGE_PHY_ADDR 3 +struct axge_frame_rxhdr { + uint32_t status; +#define AXGE_RX_L4_CSUM_ERR 0x00000001 +#define AXGE_RX_L3_CSUM_ERR 0x00000002 +#define AXGE_RX_L4_TYPE_UDP 0x00000004 +#define AXGE_RX_L4_TYPE_ICMP 0x00000008 +#define AXGE_RX_L4_TYPE_IGMP 0x0000000C +#define AXGE_RX_L4_TYPE_TCP 0x00000010 +#define AXGE_RX_L4_TYPE_MASK 0x0000001C +#define AXGE_RX_L3_TYPE_IPV4 0x00000020 +#define AXGE_RX_L3_TYPE_IPV6 0x00000040 +#define AXGE_RX_L3_TYPE_MASK 0x00000060 +#define AXGE_RX_VLAN_IND_MASK 0x00000700 +#define AXGE_RX_GOOD_PKT 0x00000800 +#define AXGE_RX_VLAN_PRI_MASK 0x00007000 +#define AXGE_RX_MBCAST 0x00008000 +#define AXGE_RX_LEN_MASK 0x1FFF0000 +#define AXGE_RX_CRC_ERR 0x20000000 +#define AXGE_RX_MII_ERR 0x40000000 +#define AXGE_RX_DROP_PKT 0x80000000 +#define AXGE_RX_LEN_SHIFT 16 +} __packed; + +#define AXGE_RXBYTES(x) (((x) & AXGE_RX_LEN_MASK) >> AXGE_RX_LEN_SHIFT) +#define AXGE_RX_ERR(x) \ + ((x) & (AXGE_RX_CRC_ERR | AXGE_RX_MII_ERR | AXGE_RX_DROP_PKT)) + struct axge_softc { struct usb_ether sc_ue; struct mtx sc_mtx;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201608180629.u7I6T7ik026391>