Bug 248494 - if_vmx(4) duplicate packets in netmap mode
Summary: if_vmx(4) duplicate packets in netmap mode
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.1-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Vincenzo Maffione
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-05 23:03 UTC by Murat
Modified: 2022-01-28 00:05 UTC (History)
6 users (show)

See Also:
vmaffione: maintainer-feedback+
vmaffione: mfc-stable12+


Attachments
ICMP DUP packets (212.23 KB, image/jpeg)
2020-08-05 23:03 UTC, Murat
no flags Details
pcap: tcp dup ACKs (9.44 KB, application/vnd.tcpdump.pcap)
2020-08-05 23:05 UTC, Murat
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Murat 2020-08-05 23:03:15 UTC
Created attachment 217042 [details]
ICMP DUP packets

scp'in a file stalls due to DUP packets if the interface is if_vmx(4); and it is opened in NETMAP mode,

Steps to reproduce:

ifconfig vmx0 -vlanhwtso -vlanhwfilter -vlanhwtag -vlanhwcsum -txcsum -rxcsum -tso4 -tso6 -lro -txcsum6 -rxcsum6

./bridge -i netmap:vmx0 -i netmap:vmx0^

Copy a file which makes use of the vmx0 interface:

scp file root@gw:

Connection will stall. 

Interestingly PINGs seem to be OK until you start scp'in the file. Then they both start getting DUP packets.  

Please see attached PCAP file and the pic.

Environment:

root@OPNsense:~ # uname -a
FreeBSD OPNsense.localdomain 12.1-STABLE FreeBSD 12.1-STABLE #2 677f5a8efda(stable/12)-dirty: Tue Aug  4 14:55:58 PDT 2020     root@client_bsd12:/usr/obj/usr/src/amd64.amd64/sys/GENERIC  amd64
Comment 1 Murat 2020-08-05 23:05:26 UTC
Created attachment 217043 [details]
pcap: tcp dup ACKs

PCAP DUP Packets
Comment 2 commit-hook freebsd_committer freebsd_triage 2020-08-06 21:33:28 UTC
A commit references this bug:

Author: vmaffione
Date: Thu Aug  6 21:32:26 UTC 2020
New revision: 363996
URL: https://svnweb.freebsd.org/changeset/base/363996

Log:
  iflib: netmap: don't increment ifl_cidx on the wrong free list

  Netmap only uses free list 0 to keep it consistent with its
  one-to-one mapping between each netmap ring and a device RX
  (or TX) queue.
  However, the current iflib_netmap_rxsync() routine was
  mistakenly updating the ifl_cidx field of both free lists.

  PR:		248494
  MFC after:	2 weeks

Changes:
  head/sys/net/iflib.c
Comment 3 Murat 2020-08-07 01:05:31 UTC
(In reply to commit-hook from comment #2)
Hi Vincenzo,

I guess the fix is not complete yet, right? Tested this commit today: still the same.
Comment 4 Vincenzo Maffione freebsd_committer freebsd_triage 2020-08-07 17:56:56 UTC
(In reply to Murat from comment #3)
That is a first fix that I did while investigating.

It's still not enough to render vmx(4) usable with netmap.
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-08-12 14:18:25 UTC
A commit references this bug:

Author: vmaffione
Date: Wed Aug 12 14:17:39 UTC 2020
New revision: 364164
URL: https://svnweb.freebsd.org/changeset/base/364164

Log:
  iflib: refactor netmap_fl_refill and fix off-by-one issue

  First, fix the initialization of the fl->ifl_rxd_idxs array,
  which was affected by an off-by-one bug.
  Once there, refactor the function to use better names for
  local variables, optimize the variable assignments, and
  merge the bus_dmamap_sync() inner loop with the outer one.

  PR:	248494
  MFC after:	3 weeks

Changes:
  head/sys/net/iflib.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2020-08-12 14:46:31 UTC
A commit references this bug:

Author: vmaffione
Date: Wed Aug 12 14:45:32 UTC 2020
New revision: 364165
URL: https://svnweb.freebsd.org/changeset/base/364165

Log:
  iflib: netmap: improve rxsync to support IFLIB_HAS_RXCQ

  For drivers with IFLIB_HAS_RXCQ set, there is a separate completion
  queue. In this case, the netmap rxsync routine needs to update
  rxq->ifr_cq_cidx in the same way it is updated by iflib_rxeof().
  This improves the situation for vmx(4) and bnxt(4) drivers, which
  use iflib and have the IFLIB_HAS_RXCQ bit set.

  PR:	248494
  MFC after:	3 weeks

Changes:
  head/sys/net/iflib.c
  head/sys/net/iflib.h
Comment 7 commit-hook freebsd_committer freebsd_triage 2020-08-21 07:53:55 UTC
A commit references this bug:

Author: vmaffione
Date: Fri Aug 21 07:52:57 UTC 2020
New revision: 364451
URL: https://svnweb.freebsd.org/changeset/base/364451

Log:
  MFC r363996

  iflib: netmap: don't increment ifl_cidx on the wrong free list

  Netmap only uses free list 0 to keep it consistent with its
  one-to-one mapping between each netmap ring and a device RX
  (or TX) queue.
  However, the current iflib_netmap_rxsync() routine was
  mistakenly updating the ifl_cidx field of both free lists.

  PR:           248494

Changes:
_U  stable/12/
  stable/12/sys/net/iflib.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2020-09-01 20:45:59 UTC
A commit references this bug:

Author: vmaffione
Date: Tue Sep  1 20:45:34 UTC 2020
New revision: 365063
URL: https://svnweb.freebsd.org/changeset/base/365063

Log:
  MFC r364164

  iflib: refactor netmap_fl_refill and fix off-by-one issue

  First, fix the initialization of the fl->ifl_rxd_idxs array,
  which was affected by an off-by-one bug.
  Once there, refactor the function to use better names for
  local variables, optimize the variable assignments, and
  merge the bus_dmamap_sync() inner loop with the outer one.

  PR:     248494

Changes:
_U  stable/12/
  stable/12/sys/net/iflib.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2020-09-01 20:48:02 UTC
A commit references this bug:

Author: vmaffione
Date: Tue Sep  1 20:47:59 UTC 2020
New revision: 365064
URL: https://svnweb.freebsd.org/changeset/base/365064

Log:
  MFC r364165

  iflib: netmap: improve rxsync to support IFLIB_HAS_RXCQ

  For drivers with IFLIB_HAS_RXCQ set, there is a separate completion
  queue. In this case, the netmap rxsync routine needs to update
  rxq->ifr_cq_cidx in the same way it is updated by iflib_rxeof().
  This improves the situation for vmx(4) and bnxt(4) drivers, which
  use iflib and have the IFLIB_HAS_RXCQ bit set.

  PR:     248494

Changes:
_U  stable/12/
  stable/12/sys/net/iflib.c
  stable/12/sys/net/iflib.h