Date: Mon, 12 May 2014 13:38:37 +0900 From: Yonghyeon PYUN <pyunyh@gmail.com> To: Michael Tuexen <Michael.Tuexen@lurchi.franken.de> Cc: FreeBSD Net <freebsd-net@freebsd.org> Subject: Re: TX Checksum offloading issue with re interfaces Message-ID: <20140512043837.GA1364@michelle.cdnetworks.com> In-Reply-To: <AB8FC94E-E93E-4CBB-94C6-4E54DFA1DF2D@lurchi.franken.de> References: <95E18108-E6DD-49EA-90B3-A9F9E5F93D20@lurchi.franken.de> <20140509014733.GB3014@michelle.cdnetworks.com> <AB8FC94E-E93E-4CBB-94C6-4E54DFA1DF2D@lurchi.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
--9jxsPFA5p3P2qPhR
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Fri, May 09, 2014 at 12:33:24PM +0200, Michael Tuexen wrote:
> On 09 May 2014, at 03:47, Yonghyeon PYUN <pyunyh@gmail.com> wrote:
>
> > On Thu, May 08, 2014 at 08:50:48PM +0200, Michael Tuexen wrote:
> >> Dear all,
> >>
> >> while testing checksum offloading of UDP packets over IP with IP options, I figured
> >> out that my card
> >>
> >> dev.re.1.%desc: RealTek 8168/8111 B/C/CP/D/DP/E/F/G PCIe Gigabit Ethernet
> >> dev.re.1.%driver: re
> >> dev.re.1.%location: slot=0 function=0 handle=\_SB_.PCI0.PE1F.LAN2
> >> dev.re.1.%pnpinfo: vendor=0x10ec device=0x8168 subvendor=0x1734 subdevice=0x1159 class=0x020000
> >> dev.re.1.%parent: pci13
> >> dev.re.1.stats: -1
> >> dev.re.1.int_rx_mod: 65
> >>
> >> computes the UDP checksum, but stores it in the packet at the place, where it would be,
> >> if there are no IP options. So it corrupts the options in the packet...
> >>
> >> I looked at sys/dev/re/if_re.c, but couldn't figure out how to fix it. Any idea?
> >>
> >
> > re(4) has a very long history on its broken TX checksum offloading.
> > So re(4) has many workarounds for known issues on several variants.
> > re(4) controllers support TX IPv4/TCP/UDP checksum offloading. For
> > 8168C/8168CP, TX IPv4 checksum offloading was disabled due to
> > generation of corrupted frames.
> > Could you show me the dmesg output(only re(4)/rgephy(4))?
>
> > The vendor uses the same PCI id for its RTL8168/8111 family chips
> > so dmesg output is necessary to know exact controller revision.
> Sure (re1 was used during the test):
> re0: <RealTek 8168/8111 B/C/CP/D/DP/E/F/G PCIe Gigabit Ethernet> port 0x8000-0x80ff mem 0xf6104000-0xf6104fff,0xf6100000-0xf6103fff irq 16 at device 0.0 on pci12
> re0: Using 1 MSI-X message
> re0: Chip rev. 0x28800000
> re0: MAC rev. 0x00200000
> miibus0: <MII bus> on re0
> rgephy0: <RTL8251 1000BASE-T media interface> PHY 1 on miibus0
> rgephy0: none, 10baseT, 10baseT-FDX, 10baseT-FDX-flow, 100baseTX, 100baseTX-FDX, 100baseTX-FDX-flow, 1000baseT, 1000baseT-master, 1000baseT-FDX, 1000baseT-FDX-master, 1000baseT-FDX-flow, 1000baseT-FDX-flow-master, auto, auto-flow
> re0: Ethernet address: 00:19:99:85:31:d9
> re1: <RealTek 8168/8111 B/C/CP/D/DP/E/F/G PCIe Gigabit Ethernet> port 0x9000-0x90ff mem 0xf5c20000-0xf5c20fff,0xf6200000-0xf620ffff irq 17 at device 0.0 on pci13
> re1: Using 1 MSI-X message
> re1: Chip rev. 0x3c800000
> re1: MAC rev. 0x00300000
> miibus1: <MII bus> on re1
> rgephy1: <RTL8169S/8110S/8211 1000BASE-T media interface> PHY 1 on miibus1
> rgephy1: none, 10baseT, 10baseT-FDX, 10baseT-FDX-flow, 100baseTX, 100baseTX-FDX, 100baseTX-FDX-flow, 1000baseT, 1000baseT-master, 1000baseT-FDX, 1000baseT-FDX-master, 1000baseT-FDX-flow, 1000baseT-FDX-flow-master, auto, auto-flow
> re1: Ethernet address: 00:19:99:7e:c7:46
>
It seems you have two variants.
re0 is RTL8168DP and re1 is RTL8168CP. Do you see the issue on
both controllers? I guess you may see the issue on re1 only since
you've posted dev.re.1 output. I've attached a diff which may
address the issue on re1 interface. If you see the issue on re0,
I have to change the diff to include RTL8168D.
--9jxsPFA5p3P2qPhR
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="re.txcsum.diff"
Index: sys/dev/re/if_re.c
===================================================================
--- sys/dev/re/if_re.c (revision 265899)
+++ sys/dev/re/if_re.c (working copy)
@@ -1619,16 +1619,18 @@ re_attach(device_t dev)
ifp->if_start = re_start;
/*
* RTL8168/8111C generates wrong IP checksummed frame if the
- * packet has IP options so disable TX IP checksum offloading.
+ * packet has IP options so disable TX checksum offloading.
*/
if (sc->rl_hwrev->rl_rev == RL_HWREV_8168C ||
sc->rl_hwrev->rl_rev == RL_HWREV_8168C_SPIN2 ||
- sc->rl_hwrev->rl_rev == RL_HWREV_8168CP)
- ifp->if_hwassist = CSUM_TCP | CSUM_UDP;
- else
+ sc->rl_hwrev->rl_rev == RL_HWREV_8168CP) {
+ ifp->if_hwassist = 0;
+ ifp->if_capabilities = IFCAP_RXCSUM | IFCAP_TSO4;
+ } else {
ifp->if_hwassist = CSUM_IP | CSUM_TCP | CSUM_UDP;
+ ifp->if_capabilities = IFCAP_HWCSUM | IFCAP_TSO4;
+ }
ifp->if_hwassist |= CSUM_TSO;
- ifp->if_capabilities = IFCAP_HWCSUM | IFCAP_TSO4;
ifp->if_capenable = ifp->if_capabilities;
ifp->if_init = re_init;
IFQ_SET_MAXLEN(&ifp->if_snd, RL_IFQ_MAXLEN);
@@ -3364,7 +3366,6 @@ re_ioctl(struct ifnet *ifp, u_long command, caddr_
struct rl_softc *sc = ifp->if_softc;
struct ifreq *ifr = (struct ifreq *) data;
struct mii_data *mii;
- uint32_t rev;
int error = 0;
switch (command) {
@@ -3453,15 +3454,9 @@ re_ioctl(struct ifnet *ifp, u_long command, caddr_
if ((mask & IFCAP_TXCSUM) != 0 &&
(ifp->if_capabilities & IFCAP_TXCSUM) != 0) {
ifp->if_capenable ^= IFCAP_TXCSUM;
- if ((ifp->if_capenable & IFCAP_TXCSUM) != 0) {
- rev = sc->rl_hwrev->rl_rev;
- if (rev == RL_HWREV_8168C ||
- rev == RL_HWREV_8168C_SPIN2 ||
- rev == RL_HWREV_8168CP)
- ifp->if_hwassist |= CSUM_TCP | CSUM_UDP;
- else
- ifp->if_hwassist |= RE_CSUM_FEATURES;
- } else
+ if ((ifp->if_capenable & IFCAP_TXCSUM) != 0)
+ ifp->if_hwassist |= RE_CSUM_FEATURES;
+ else
ifp->if_hwassist &= ~RE_CSUM_FEATURES;
reinit = 1;
}
--9jxsPFA5p3P2qPhR--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140512043837.GA1364>