> Date: Thu, 27 Sep 2012 09:54:35 +1000
> From: Darren Tucker <dtuc...@zip.com.au>
>
> Hi all.
>
> This diff adds hardware 802.1q VLAN tagging support to vr(4) (just
> tag/untag, it doesn't do anything with the VLAN CAM filters).
>
> As far as I know, the capability is only in the VT6105M Rhine III chip
> which is used, amongst other places, in the pcengines ALIX machines.
>
> If anyone has a vr(4) (any revision), especially if you use VLANs,
> I'd be interested to hear if this works for you.
>
> I'm not a kernel guy, so it's quite possible I made some basic mistake
> in there somewhere, but it works for me on an ALIX:
>
> vr0 at pci0 dev 9 function 0 "VIA VT6105M RhineIII" rev 0x96: irq 10, address
> 00:0d:b9:7e:21:5c
> ukphy0 at vr0 phy 1: Generic IEEE 802.3u media interface, rev. 3: OUI
> 0x004063, model 0x0034
>
> $ ifconfig vr0 hwfeatures | grep hwfeatures
>
> hwfeatures=8037<CSUM_IPv4,CSUM_TCPv4,CSUM_UDPv4,VLAN_MTU,VLAN_HWTAGGING,WOL>
>
> As an aside, I found what I believe is a latent bug in the existing
> driver. When it's done with the TX descriptor, it sets the "owner"
> bit to tell the chip that it's good to go:
>
> #define VR_TXSTAT_OWN 0x80000000
> #define VR_TXOWN(x) x->vr_ptr->vr_status
>
> VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
>
> which expands to:
>
> cur_tx->vr_ptr->vr_status = htole32(0x80000000);
>
> which zeroes the 31 least significant bits in the first word of the
> TX descriptor. Sadly, this includes the VLAN ID, which caused me some
> head-scratching trying to figure out why my tagged frames were all in
> VLAN 0.
>
> On the plus side, nothing else currently uses those bits, so right
> now it doesn't matter. My questions is: why hide that behind a macro?
> or at least, if you're going to, why not go the whole hog and put the
> entire thing in there?
>
> #define VR_TXOWN(x) ((x)->vr_ptr->vr_status |= htole32(VR_TXSTAT_OWN))
Yes. I consider using macros in the LHS of an assignment bad style.
But I'd just drop the macro entirely. I think having the code show
explicitly that vr_status is getting changed is better, and would
probably have prevented the latent bug you mention above.
Diff looks good to me, but I don't have the hardware to test this.
ok kettenis@
> Index: sys/dev/pci/if_vr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 if_vr.c
> --- sys/dev/pci/if_vr.c 30 Jan 2012 09:11:30 -0000 1.114
> +++ sys/dev/pci/if_vr.c 26 Sep 2012 23:13:45 -0000
> @@ -62,6 +62,7 @@
> */
>
> #include "bpfilter.h"
> +#include "vlan.h"
>
> #include <sys/param.h>
> #include <sys/systm.h>
> @@ -83,6 +84,11 @@
> #include <net/if_dl.h>
> #include <net/if_media.h>
>
> +#if NVLAN > 0
> +#include <net/if_types.h>
> +#include <net/if_vlan_var.h>
> +#endif
> +
> #if NBPFILTER > 0
> #include <net/bpf.h>
> #endif
> @@ -633,6 +639,12 @@ vr_attach(struct device *parent, struct
> if (sc->vr_quirks & VR_Q_CSUM)
> ifp->if_capabilities |= IFCAP_CSUM_IPv4|IFCAP_CSUM_TCPv4|
> IFCAP_CSUM_UDPv4;
> +
> +#if NVLAN > 0
> + /* if the hardware can do VLAN tagging, say so. */
> + if (sc->vr_quirks & VR_Q_HWTAG)
> + ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
> +#endif
> #ifndef SMALL_KERNEL
> if (sc->vr_revid >= REV_ID_VT3065_A) {
> ifp->if_capabilities |= IFCAP_WOL;
> @@ -880,6 +892,23 @@ vr_rxeof(struct vr_softc *sc)
> cur_rx->vr_map->dm_mapsize, BUS_DMASYNC_POSTREAD);
> bus_dmamap_unload(sc->sc_dmat, cur_rx->vr_map);
>
> +#if NVLAN > 0
> + /*
> + * If there's a tagged packet, the 802.1q header will be at the
> + * 4-byte boundary following the CRC. There will be 2 bytes
> + * TPID (0x8100) and 2 bytes TCI (including VLAN ID).
> + * This isn't in the data sheet.
> + */
> + if (rxctl & VR_RXCTL_TAG) {
> + u_int16_t vtag;
> + int offset = ((total_len + 3) & ~3) + 2;
> +
> + bcopy(m->m_data + offset, &vtag, sizeof(vtag));
> + m->m_pkthdr.ether_vtag = ntohs(vtag);
> + m->m_flags |= M_VLANTAG;
> + }
> +#endif
> +
> /*
> * The VIA Rhine chip includes the CRC with every
> * received frame, and there's no way to turn this
> @@ -1002,7 +1031,7 @@ vr_txeof(struct vr_softc *sc)
> sc->vr_flags |= VR_F_RESTART;
> break;
> }
> - VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
> + VR_TXOWN(cur_tx) |= htole32(VR_TXSTAT_OWN);
> CSR_WRITE_4(sc, VR_TXADDR, cur_tx->vr_paddr);
> break;
> }
> @@ -1170,6 +1199,16 @@ vr_encap(struct vr_softc *sc, struct vr_
> struct mbuf *m_new = NULL;
> u_int32_t vr_flags = 0, vr_status = 0;
>
> +#if NVLAN > 0
> + /* Tell chip to insert VLAN tag if needed. */
> + if (m_head->m_flags & M_VLANTAG) {
> + u_int32_t vtag = m_head->m_pkthdr.ether_vtag;
> + vtag = (vtag << VR_TXSTAT_PQSHIFT) & VR_TXSTAT_PQMASK;
> + vr_status |= vtag;
> + vr_flags |= VR_TXCTL_INSERTTAG;
> + }
> +#endif
> +
> if (sc->vr_quirks & VR_Q_CSUM) {
> if (m_head->m_pkthdr.csum_flags & M_IPV4_CSUM_OUT)
> vr_flags |= VR_TXCTL_IPCSUM;
> @@ -1274,7 +1313,7 @@ vr_start(struct ifnet *ifp)
> break;
> }
>
> - VR_TXOWN(cur_tx) = htole32(VR_TXSTAT_OWN);
> + VR_TXOWN(cur_tx) |= htole32(VR_TXSTAT_OWN);
>
> #if NBPFILTER > 0
> /*
> @@ -1394,6 +1433,11 @@ vr_init(void *xsc)
> * Program promiscuous mode and multicast filters.
> */
> vr_iff(sc);
> +
> +#if NVLAN > 0
> + if (ifp->if_capabilities & IFCAP_VLAN_HWTAGGING)
> + VR_SETBIT(sc, VR_TXCFG, VR_TXCFG_TXTAGEN);
> +#endif
>
> /*
> * Load the address of the RX list.
> Index: sys/dev/pci/if_vrreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_vrreg.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 if_vrreg.h
> --- sys/dev/pci/if_vrreg.h 5 Jan 2012 19:08:25 -0000 1.30
> +++ sys/dev/pci/if_vrreg.h 26 Sep 2012 23:13:45 -0000
> @@ -83,6 +83,9 @@
> #define VR_MPA_CNT 0x7C
> #define VR_CRC_CNT 0x7E
> #define VR_STICKHW 0x83
> +#define VR_CAMMASK 0x88 /* 4 bytes */
> +#define VR_CAMCTRL 0x92
> +#define VR_CAMADDR 0x93
>
> /* Misc Registers */
> #define VR_MISC_CR1 0x81
> @@ -342,6 +345,14 @@
> #define VR_BCR1_VLANFILT_ENB 0x80 /* 6105M */
>
> /*
> + * CAM Control registers (VT6105M)
> + */
> +#define VR_CAMCTRL_WREN 0x01
> +#define VR_CAMCTRL_VCAMSEL 0x02
> +#define VR_CAMCTRL_WRITE 0x04
> +#define VR_CAMCTRL_READ 0x08
> +
> +/*
> * Rhine TX/RX list structure.
> */
>
> @@ -404,6 +415,7 @@ struct vr_desc {
> #define VR_TXSTAT_ERRSUM 0x00008000
> #define VR_TXSTAT_PQMASK 0x7FFF0000
> #define VR_TXSTAT_OWN 0x80000000
> +#define VR_TXSTAT_PQSHIFT 16
>
> #define VR_TXCTL_BUFLEN 0x000007FF
> #define VR_TXCTL_BUFLEN_EXT 0x00007800
>
> --
> Darren Tucker (dtucker at zip.com.au)
> GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
> Good judgement comes with experience. Unfortunately, the experience
> usually comes from bad judgement.
