Page MenuHomeFreeBSD
Differential D19357

PFIL_MEMPTR support for ipfw link level hook
ClosedPublic
Actions

Authored by glebius on Feb 25 2019, 9:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 30, 12:33 AM
Unknown Object (File)
Mon, Dec 29, 8:05 PM
Unknown Object (File)
Mon, Dec 29, 4:25 PM
Unknown Object (File)
Sun, Dec 28, 11:33 AM
Unknown Object (File)
Fri, Dec 26, 7:50 PM
Unknown Object (File)
Fri, Dec 26, 11:12 AM
Unknown Object (File)
Thu, Dec 25, 8:46 AM
Unknown Object (File)
Dec 15 2025, 6:49 PM
View All Files
Subscribers
farrokhi
imp
rgrimes

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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22732
Build 21828: arc lint + arc unit

Event Timeline

glebius created this revision.Feb 25 2019, 9:46 PM
Herald added a subscriber: farrokhi. · View Herald TranscriptFeb 25 2019, 9:46 PM
Harbormaster completed remote builds in B22732: Diff 54378.Feb 25 2019, 9:46 PM
ae added inline comments.Mar 11 2019, 9:50 AM
sys/dev/virtio/network/if_vtnet.c
1837

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

sys/netpfil/ipfw/ip_fw2.c
1396

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

1757

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 marked an inline comment as done.Mar 11 2019, 9:58 PM
glebius added inline comments.
sys/dev/virtio/network/if_vtnet.c
1837

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

sys/netpfil/ipfw/ip_fw2.c
1396

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

1757

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

rgrimes added a subscriber: rgrimes.Mar 11 2019, 10:05 PM
rgrimes added inline comments.
sys/netpfil/ipfw/ip_fw2.c
1757

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 updated this revision to Diff 54945.Mar 11 2019, 10:27 PM
glebius marked an inline comment as done.

Rebase since some changes are already in head.

Harbormaster completed remote builds in B23016: Diff 54945.Mar 11 2019, 10:27 PM
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

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.

glebius updated this revision to Diff 54946.Mar 11 2019, 10:48 PM

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

Harbormaster completed remote builds in B23017: Diff 54946.Mar 11 2019, 10:48 PM
glebius updated this revision to Diff 54948.Mar 11 2019, 11:28 PM

Parse VLAN headers.

Harbormaster completed remote builds in B23019: Diff 54948.Mar 11 2019, 11:28 PM
glebius updated this revision to Diff 54949.Mar 11 2019, 11:47 PM

Style - no functional change.

Harbormaster completed remote builds in B23020: Diff 54949.Mar 11 2019, 11:47 PM
ae added inline comments.Mar 12 2019, 8:38 AM
sys/netpfil/ipfw/ip_fw2.c
1435

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

glebius updated this revision to Diff 55001.Mar 12 2019, 8:28 PM
  • Use existing difference between ip and eh pointers to calculate
Harbormaster completed remote builds in B23049: Diff 55001.Mar 12 2019, 8:28 PM
glebius marked an inline comment as done.Mar 12 2019, 8:29 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 14 2019, 10:52 PM
Closed by commit rS345166: PFIL_MEMPTR for ipfw link level hook (authored by glebius). · Explain Why
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptMar 14 2019, 10:52 PM

Revision Contents
Changeset List

PathSize
sys/
dev/
virtio/
network/
50 lines
1 line
net/
18 lines
netgraph/
10 lines
netinet/
2 lines
8 lines
7 lines
netpfil/
ipfw/
21 lines
17 lines
178 lines
51 lines
9 lines
35 lines
7 lines
157 lines
57 lines
pf/
4 lines

Diff 54378

View Options

sys/dev/virtio/network/if_vtnet.c

Loading...
View Options

sys/dev/virtio/network/if_vtnetvar.h

Loading...
View Options

sys/net/pfil.c

Loading...
View Options

sys/netgraph/ng_ipfw.c

Loading...
View Options

sys/netinet/ip_divert.c

Loading...
View Options

sys/netinet/ip_var.h

Loading...
View Options

sys/netinet/raw_ip.c

Loading...
View Options

sys/netpfil/ipfw/ip_dn_io.c

Loading...
View Options

sys/netpfil/ipfw/ip_dn_private.h

Loading...
View Options

sys/netpfil/ipfw/ip_fw2.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_bpf.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_dynamic.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_log.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_nat.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_pfil.c

Loading...
View Options

sys/netpfil/ipfw/ip_fw_private.h

Loading...
View Options

sys/netpfil/pf/pf.c

Loading...