Page MenuHomeFreeBSD

PFIL_MEMPTR support for ipfw link level hook
ClosedPublic

Authored by glebius on Feb 25 2019, 9:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 18, 6:42 PM
Unknown Object (File)
Mon, Mar 18, 6:42 PM
Unknown Object (File)
Mon, Mar 18, 6:42 PM
Unknown Object (File)
Mon, Mar 18, 6:42 PM
Unknown Object (File)
Mon, Mar 18, 6:42 PM
Unknown Object (File)
Feb 12 2024, 4:41 PM
Unknown Object (File)
Jan 23 2024, 1:52 PM
Unknown Object (File)
Jan 20 2024, 2:24 AM

Details

Reviewers
ae
Group Reviewers
network
Commits
rS345166: PFIL_MEMPTR for ipfw link level hook
Summary

This commit brings support PFIL_MEMPTR support to ipfw link level hook.

This hook is expected to be used on NIC bound pfil heads, which provide
memory pointers rather than mbufs. As for now pfil_fake_mbuf() is used
to affiliate that. Skipping pfil_fake_mbuf() and working with memory
pointer directly inside ipfw gives measureable performance boost.

The patch in this review is rather large, so I'd suggest to look
at history of this branch:

https://github.com/glebius/FreeBSD/commits/pfil

The most important commit is the topmost:

0512e8e3de490b3ab2497c46373acf723dd3f6b9

This is the most crucial code that I want to be reviewed.

The following commits are preparatory steps:

1ce76e69e0219c31b3deccd4b2f9751987917763
f3941316b74a275292d8125c77dd114449481d77
45ab1e7e3aaaae323864ab56e4bbae4d43e0e4d5
72029a8fbfce40159a7006ed10eddb4400230ef8
b24f3ee15e55643aa2c6871b450f468ec4766a7f
17692f9c2cf07275fd680bc67d87cd5b099b9236

Commit bfb6a4ff73f1b4dd99ca5165080a63cc6e558a44 is a bugfix and isn't
directly related to this review. Commit isn't directly related as well a014887dd7c60ec96956bcc5cb7fec95a5a6f621.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/dev/virtio/network/if_vtnet.c
1837 ↗(On Diff #54378)

Is it safe to call vtnet_rxq_discard_buf() just after if_input()?

sys/netpfil/ipfw/ip_fw2.c
1396 ↗(On Diff #54378)

Is it expected that ipfw_chk() will always get from pfil ethernet frame without VLAN tags?

1757 ↗(On Diff #54378)

This probably can break something. mbuf can have initialized oif and m_pkthdr.rcvif in the same time, this happens when we are processing forwarded packet.

glebius added inline comments.
sys/dev/virtio/network/if_vtnet.c
1837 ↗(On Diff #54378)

This part of patch has changed. I will update pull request soon.

sys/netpfil/ipfw/ip_fw2.c
1396 ↗(On Diff #54378)

That's a great question! Is that responsibility of a driver or of a packet filter. We will ponder on that.

1757 ↗(On Diff #54378)

Is it possible to construct a rule that would match on oif and iif at the same time?

rgrimes added inline comments.
sys/netpfil/ipfw/ip_fw2.c
1757 ↗(On Diff #54378)

Not only possible, in use. You can make decisions about a packet exiting based on if it arrived via a specific if.
ipfw add 1234 allow ip from any to any in via em0 out via em1

glebius marked an inline comment as done.

Rebase since some changes are already in head.

glebius marked an inline comment as not done.Mar 11 2019, 10:43 PM
glebius added inline comments.
sys/netpfil/ipfw/ip_fw2.c
1757 ↗(On Diff #54378)

This particular syntax won't match anything, since "via" always uses either of two interfaces. Also, a rule having "in" and "out" at the same time shoudn't match as well.

However, this rule correctly matches on two interfaces indeed:

ipfw add allow ip from any to any recv em0 out xmit em1

"out" is optional here. So, something needs to be fixed with my patch.

Optionally use m->m_pkthdr.rcvif for iif on output packets.

Style - no functional change.

sys/netpfil/ipfw/ip_fw2.c
1439 ↗(On Diff #54949)

It seems you forgot about ETHER_VLAN_ENCAP_LEN :-)
I think you can use difference between eh and ip pointers.

  • Use existing difference between ip and eh pointers to calculate
This revision was not accepted when it landed; it landed in state Needs Review.Mar 14 2019, 10:52 PM
This revision was automatically updated to reflect the committed changes.