Another mistake
On Wed, Oct 12, 2022 at 01:45:30PM +0200, Moritz Buhl wrote:
> On Tue, Oct 11, 2022 at 04:16:15PM +0100, Stuart Henderson wrote:
> > On 2022/10/11 15:03, Moritz Buhl wrote:
> > > Here is a new diff for checksum offloading (ipv4, udp, tcp) for em(4).
> > >
> > > The previous diff didn't implement hardware vlan tagging for >em82578
> > > which should result in variable ethernet header lengths and thus
> > > wrong checksums inserted at wrong places.
> > >
> > > The diff below addresses this.
> > > I would appreciate further testing reports with different controllers.
> > >
> > > mbuhl
> >
> > I tried this on my laptop which has I219-V em (I run it in a trunk
> > with iwm). It breaks tx (packets don't show up on the other side).
> > rx seems ok.
>
> The following diff will restrict the usage of the advanced
> descriptors to 82575, 82576, i350 and i210, and fix what the
> last diff broke for i219.
>
> Index: dev/pci/if_em.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_em.c,v
> retrieving revision 1.362
> diff -u -p -r1.362 if_em.c
> --- dev/pci/if_em.c 23 Jun 2022 09:38:28 -0000 1.362
> +++ dev/pci/if_em.c 11 Oct 2022 16:05:43 -0000
> @@ -37,6 +37,8 @@ POSSIBILITY OF SUCH DAMAGE.
> #include <dev/pci/if_em.h>
> #include <dev/pci/if_em_soc.h>
>
> +#include <netinet/ip6.h>
> +
> /*********************************************************************
> * Driver version
> *********************************************************************/
> @@ -278,6 +280,8 @@ void em_receive_checksum(struct em_softc
> struct mbuf *);
> u_int em_transmit_checksum_setup(struct em_queue *, struct mbuf *,
> u_int,
> u_int32_t *, u_int32_t *);
> +u_int em_tx_ctx_setup(struct em_queue *, struct mbuf *, u_int,
> u_int32_t *,
> + u_int32_t *);
> void em_iff(struct em_softc *);
> void em_update_link_status(struct em_softc *);
> int em_get_buf(struct em_queue *, int);
> @@ -1220,10 +1224,9 @@ em_encap(struct em_queue *que, struct mb
> BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
> }
>
> - if (sc->hw.mac_type >= em_82543 && sc->hw.mac_type != em_82575 &&
> - sc->hw.mac_type != em_82576 &&
> - sc->hw.mac_type != em_82580 && sc->hw.mac_type != em_i210 &&
> - sc->hw.mac_type != em_i350) {
> + if (sc->hw.mac_type >= em_82575 && sc->hw.mac_type <= em_i210) {
> + used += em_tx_ctx_setup(que, m, head, &txd_upper, &txd_lower);
> + } else if (sc->hw.mac_type >= em_82543) {
> used += em_transmit_checksum_setup(que, m, head,
> &txd_upper, &txd_lower);
> } else {
> @@ -1278,7 +1281,7 @@ em_encap(struct em_queue *que, struct mb
>
> #if NVLAN > 0
> /* Find out if we are in VLAN mode */
> - if (m->m_flags & M_VLANTAG) {
> + if (m->m_flags & M_VLANTAG && sc->hw.mac_type < em_82575) {
I missed this spot, it should be
if (m->m_flags & M_VLANTAG && (sc->hw.mac_type < em_82575 ||
sc->hw.mac_type > em_i210)) {
