Re: kern/53562: bridge(4) breaks segmentation / TX checksum offloading

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

On 2018/08/30 18:25, rokuyama%rk.phys.keio.ac.jp@localhost wrote:
> > Number:         53562
> > Category:       kern
> > Synopsis:       bridge(4) breaks segmentation / TX checksum offloading
> > Confidential:   no
> > Severity:       serious
> > Priority:       medium
> > Responsible:    kern-bug-people
> > State:          open
> > Class:          sw-bug
> > Submitter-Id:   net
> > Arrival-Date:   Thu Aug 30 09:25:00 +0000 2018
> > Originator:     Rin Okuyama
> > Release:        8.99.24
> > Organization:
> School of Science and Technology, Meiji University
> > Environment:
> NetBSD rpi3b 8.99.24 NetBSD 8.99.24 (RPI3-64) #24: Thu Aug 30 17:39:24 JST 2018  rin@latipes:/var/build/src/sys/arch/evbarm/compile/RPI3-64 evbarm aarch64
> > Description:
> If a network interface is added to bridge(4), segmentation or TX
> checksum offloading do not work. This is because csum_flags are
> cleared in bridge_enqueue():
>
> https://nxr.netbsd.org/xref/src/sys/net/if_bridge.c#1401
>
>    1391  void
>    1392  bridge_enqueue(struct bridge_softc *sc, struct ifnet *dst_ifp, struct mbuf *m,
>    1393      int runfilt)
>    1394  {
>    ....
>    1398          /*
>    1399           * Clear any in-bound checksum flags for this packet.
>    1400           */
>    1401          m->m_pkthdr.csum_flags = 0;
> > How-To-Repeat:
> Enable segmentation or TX checksum offloading for a network interface,
> and add it to bridge(4). Then, data transmission starts to fail.
> > Fix:
> Not known.

Is the following change correct?

Index: if_bridge.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_bridge.c,v
retrieving revision 1.156
diff -u -p -r1.156 if_bridge.c
--- if_bridge.c	25 May 2018 04:40:27 -0000	1.156
+++ if_bridge.c	3 Sep 2018 08:51:23 -0000
@@ -1395,11 +1395,6 @@ bridge_enqueue(struct bridge_softc *sc,
 	int len, error;
 	short mflags;
 
-	/*
-	 * Clear any in-bound checksum flags for this packet.
-	 */
-	m->m_pkthdr.csum_flags = 0;
-
 	if (runfilt) {
 		if (pfil_run_hooks(sc->sc_if.if_pfil, &m,
 		    dst_ifp, PFIL_OUT) != 0) {
@@ -1644,6 +1639,11 @@ bridge_forward(struct bridge_softc *sc,
 	if ((sc->sc_if.if_flags & IFF_RUNNING) == 0)
 		return;
 
+	/*
+	 * Clear any in-bound checksum flags for this packet.
+	 */
+	m->m_pkthdr.csum_flags = 0;
+
 	src_if = m_get_rcvif_psref(m, &psref_src);
 	if (src_if == NULL) {
 		/* Interface is being destroyed? */


--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)