Re: hardware VLAN tagging for vr(4)
On Mon, Jan 14, 2013 at 02:42:54PM +1100, Darren Tucker wrote:
Resurrecting this, I've now fixed the problem I introduced when I merged
in your changes and have tested it.
Updated diff now incorporating feedback from brad jsing mikeb and
probably others. Having corrected for the mistake of benchmarking
against a POOL_DEBUG kernel, this stil seems marginally better (71.3%
cpu vs 71.5%, which is admittedly within the margin of error).
Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.122
diff -u -p -r1.122 if_vr.c
--- dev/pci/if_vr.c 16 Jan 2013 03:21:14 - 1.122
+++ dev/pci/if_vr.c 16 Jan 2013 04:04:34 -
@@ -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
@@ -632,6 +638,12 @@ vr_attach(struct device *parent, struct
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;
@@ -899,6 +911,7 @@ vr_rxeof(struct vr_softc *sc)
#endif
ifp-if_ipackets++;
+
if (sc-vr_quirks VR_Q_CSUM
(rxstat VR_RXSTAT_FRAG) == 0
(rxctl VR_RXCTL_IP) != 0) {
@@ -911,6 +924,21 @@ vr_rxeof(struct vr_softc *sc)
M_UDP_CSUM_IN_OK;
}
+#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) {
+ int offset = ((total_len + 3) ~3) + ETHER_CRC_LEN + 2;
+ m-m_pkthdr.ether_vtag = htons(*(u_int16_t *)
+ ((u_int8_t *)m-m_data + offset));
+ m-m_flags |= M_VLANTAG;
+ }
+#endif
+
#if NBPFILTER 0
/*
* Handle BPF listeners. Let the BPF user see the packet.
@@ -1229,6 +1257,15 @@ vr_encap(struct vr_softc *sc, struct vr_
c-vr_mbuf = m_head;
txmap = c-vr_map;
for (i = 0; i txmap-dm_nsegs; i++) {
+#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_ctl |= htole32(VR_TXCTL_INSERTTAG);
+ }
+#endif
if (i != 0)
*cp = c = c-vr_nextdesc;
f = c-vr_ptr;
@@ -1397,6 +1434,9 @@ vr_init(void *xsc)
VR_CLRBIT(sc, VR_TXCFG, VR_TXCFG_TX_THRESH);
VR_SETBIT(sc, VR_TXCFG, VR_TXTHRESH_STORENFWD);
+
+ if (ifp-if_capabilities IFCAP_VLAN_HWTAGGING)
+ VR_SETBIT(sc, VR_TXCFG, VR_TXCFG_TXTAGEN);
/* Init circular RX list. */
if (vr_list_rx_init(sc) == ENOBUFS) {
--
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.
Re: hardware VLAN tagging for vr(4)
Resurrecting this, I've now fixed the problem I introduced when I merged
in your changes and have tested it.
The performance on my ALIX routing from a VLAN to another non-VLAN
interface, iperf TCP went up a little (the CPU is saturated at this
point):
baseline: RX 80.0 Mbits/sec TX 65.2 Mbits/sec
hwtagging: RX 80.4 Mbits/sec TX 73.2 Mbits/sec
ok?
Index: dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.121
diff -u -p -r1.121 if_vr.c
--- dev/pci/if_vr.c 1 Dec 2012 09:55:03 - 1.121
+++ dev/pci/if_vr.c 14 Jan 2013 03:30:55 -
@@ -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
@@ -632,6 +638,12 @@ vr_attach(struct device *parent, struct
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;
@@ -874,6 +886,21 @@ 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) {
+ int offset = ((total_len + 3) ~3) + 2;
+ m-m_pkthdr.ether_vtag = htons(*(u_int16_t *)
+ ((u_int8_t *)m-m_data + offset));
+ 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
@@ -1223,12 +1250,20 @@ vr_encap(struct vr_softc *sc, struct vr_
if (m_new != NULL) {
m_freem(m_head);
-
c-vr_mbuf = m_new;
} else
c-vr_mbuf = m_head;
txmap = c-vr_map;
for (i = 0; i txmap-dm_nsegs; i++) {
+#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_ctl |= htole32(VR_TXCTL_INSERTTAG);
+ }
+#endif
if (i != 0)
*cp = c = c-vr_nextdesc;
f = c-vr_ptr;
@@ -1254,7 +1289,7 @@ vr_encap(struct vr_softc *sc, struct vr_
sc-vr_cdata.vr_tx_cnt++;
}
- /* Set EOP on the last desciptor */
+ /* Set EOP on the last descriptor */
f-vr_ctl |= htole32(VR_TXCTL_LASTFRAG | VR_TXCTL_FINT);
return (0);
@@ -1395,6 +1430,11 @@ vr_init(void *xsc)
VR_CLRBIT(sc, VR_TXCFG, VR_TXCFG_TX_THRESH);
VR_SETBIT(sc, VR_TXCFG, VR_TXTHRESH_STORENFWD);
+
+#if NVLAN 0
+ if (ifp-if_capabilities IFCAP_VLAN_HWTAGGING)
+ VR_SETBIT(sc, VR_TXCFG, VR_TXCFG_TXTAGEN);
+#endif
/* Init circular RX list. */
if (vr_list_rx_init(sc) == ENOBUFS) {
Index: dev/pci/if_vrreg.h
===
RCS file: /cvs/src/sys/dev/pci/if_vrreg.h,v
retrieving revision 1.32
diff -u -p -r1.32 if_vrreg.h
--- dev/pci/if_vrreg.h 20 Oct 2012 16:12:22 - 1.32
+++ dev/pci/if_vrreg.h 14 Jan 2013 03:30:55 -
@@ -83,6 +83,9 @@
#define VR_MPA_CNT 0x7C
#define VR_CRC_CNT 0x7E
#define VR_STICKHW 0x83
+#define VR_CAMMASK 0x88 /* length 4 bytes */
+#define VR_CAMCTRL 0x92
+#define VR_CAMADDR 0x93
/* Misc Registers */
#define VR_MISC_CR10x81
@@ -342,6 +345,14 @@
#define VR_BCR1_VLANFILT_ENB 0x80/* 6105M */
/*
+ * CAM Control registers (VT6105M)
+ */
+#define VR_CAMCTRL_WREN0x01
+#define VR_CAMCTRL_VCAMSEL 0x02
+#define VR_CAMCTRL_WRITE 0x04
+#define VR_CAMCTRL_READ0x08
+
+/*
* Rhine TX/RX list structure.
*/
@@ -404,6 +415,7 @@ struct vr_desc {
#define VR_TXSTAT_ERRSUM 0x8000
#define VR_TXSTAT_PQMASK 0x7FFF
#define
Re: hardware VLAN tagging for vr(4)
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=8037CSUM_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 0x8000
#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(0x8000);
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 - 1.114
+++ sys/dev/pci/if_vr.c 26 Sep 2012 23:13:45 -
@@ -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
hardware VLAN tagging for vr(4)
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=8037CSUM_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 0x8000
#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(0x8000);
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))
Thanks.
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 - 1.114
+++ sys/dev/pci/if_vr.c 26 Sep 2012 23:13:45 -
@@ -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 |=
