Netmap 11 to 12 fix

[muratbalaban43]

Although, there are no visible API changes, moving from API version
11 to 12, for every rx/tx ring that is being opened, one needs to
explicitly call NETMAP_IF to get the fresh address of the netmap_if
struct.

When the netmap-rework branch is done, it will also handle the
situation, but anyhow, this is a quick fix for 4.1.3 so that Suricata
can still happily run on FreeBSD 12 & 11 newcoming releases, since
they all come with the new netmap code.

Contributed by: Sunny Valley Networks

Attn: @inliniac

Make sure these boxes are signed before submitting your Pull Request -- thank you.

• I have read the contributing guide lines at https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Contributing
• I have signed the Open Information Security Foundation contribution agreement at https://suricata-ids.org/about/contribution-agreement/
• I have updated the user guide (in doc/userguide/) to reflect the changes made (if applicable)

Link to redmine ticket:

Describe changes:

PRScript output (if applicable):

[vmaffione]

Hi,
No fix should be necessary, just a recompilation.
As you say, there is no visible change for the legacy API (NIOCREGIF).
If this is not true, it may be that support for the legacy API has some bugs.
Could you please share a minimal configuration of suricata, so that I can try myself and see where the problem is?

[muratbalaban43]

@vmaffione , here is the relavent netmap section. Suricata-netmap bridges igb0 hw <-> sw rings:

netmap:

• interface: default
  threads: auto
  copy-mode: ips
  disable-promisc: yes # promiscuous mode
  checksum-checks: auto

• interface: igb0
  copy-iface: igb0+

• interface: igb0+
  copy-iface: igb0

[vmaffione]

Is this a complete configuration that I can just copy-paste and it should work?
I'm sorry, I've never used suricata before.

[muratbalaban43]

@vmaffione , no worries. Attached. Make sure you rename it to suricata.yaml (github was not happy with .yaml extension).

suricata.yaml.txt

[vmaffione]

Thanks a lot, I will try asap.

[vmaffione]

Hi,
I tried to install suricata from these github sources, on FreeBSD 12.0-RELEASE, and without this patch.
I use the following commands to run suricata over an em interface:

sudo ifconfig em1 up -arp promisc -rxcsum -txcsum -rxcsum6 -txcsum6 -tso -tso4 -tso6 -lro -vlanmtu -vlanhwtag -vlanhwfilter -vlanhwtso
sudo suricata -c /etc/suricata/suricata.yaml --netmap=em1 -v

This is the netmap section of my suricata.yaml:

netmap:
  - interface: default
    threads: auto
    copy-mode: ips
    disable-promisc: yes #  promiscuous mode
    checksum-checks: auto

  - interface: em1
    copy-iface: em1+

  - interface: em1+
    copy-iface: em1

and I see packets being captured

[100078] 3/2/2019 -- 14:01:59 - (util-device.c:329) <Notice> (LiveDeviceListClean) -- Stats for 'em1':  pkts: 4892, drop: 0 (0.00%), invalid chksum: 0

So why is this patch needed?

[vmaffione]

(more information here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230465#c14)

[fichtner]

You're not using em1+ likely because you stuff --netmap=em1 from the command line

[vmaffione]

My bad, sorry. I'm not familiar with suricata.
The question is, though, what should I do to reproduce the issue?

[fichtner]

Give the yaml to suricata, do not mention interfaces on the command line. Run traffic through em1. It should block instead of flowing through when suricata is running (ping is enough to reproduce).

[muratbalaban43]

@vmaffione , try adding --netmap (without interface name)

suricata -c suricata.yaml --netmap -s signatures.rules

[vmaffione]

Yes, indeed, thanks.
I'm able to reproduce the issue :)

[muratbalaban43]

All welcome 👍

[vmaffione]

I see the problem now, and your patch is the right fix. In the original netmap implementation, you could only open all the RX/TX/host rings, so all or none.
This means that the struct netmap_if always had all the ring_ofs entries valid, e.g. pointing to all the TX, RX and host rings.
And suricata is written making this assumption.

However, with the introduction of the "partial opening" feature, that was not the case anymore.
If you open netmap:ix-3, for example, only the fourth TX ring and the fourth RX ring are reachable through the netmap_if. The others just point to a special invalid ring with all the slots pointing to a special invalid buffer, which means that you can't use those rings.
In other words, each NIOCREGIF registration allocates an independent instance of struct netmap_if, and thus if you separately open hw rings and sw rings you'll get two struct netmap_if, one with the valid hw rings, and the other with valid sw rings. That's why you need to use NETMAP_IF after each NIOCREGIF.

[vmaffione]

Btw this change of behavior happened way before switching from 11 to 12, so the latter control API change is not responsible for this issue (as the title suggests).

[victorjulien]

@vmaffione thanks a lot for your feedback, appreciate it a lot that you've taken the time for this!

@muratbalaban43 can you submit a new PR with the suggested updates? Also the coding style was a bit off and the commit message needs to be cleaned up a bit.

Please see https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Github_work_flow#Create-a-new-branch-for-incorporating-feedback
and
https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Code_Submission_Quality_Criteria

Thanks!

[vmaffione]

@victorjulien no problems, I want suricata to work properly on netmap.

Btw, is there any particular reason why you open each netmap ring using a separate instance of /dev/netmap and a separate NIOCREGIF?
From what I can see from your code this is unnecessary, and you can use a single NIOCREGIF, which would make the code simpler (and also remove the issue btw). Maybe I'm missing something?
I could do the change, if you wish.

[vmaffione]

Of course you do that because you want to process each ring in a different thread.. please ignore my last message, and let's go for Murat's patch :)

[muratbalaban43]

@vmaffione Thanks for the comments. @victorjulien , I'll be creating another request. Thank you for the pointers.