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

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

The following reply was made to PR kern/53562; it has been noted by GNATS.

From: Masanobu SAITOH <msaitoh%execsw.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, kern-bug-people%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Cc: msaitoh%execsw.org@localhost
Subject: Re: kern/53562: bridge(4) breaks segmentation / TX checksum
 offloading
Date: Mon, 3 Sep 2018 17:52:52 +0900

 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)