pf: also apply dummynet to route-to/dup-to packets
ClosedPublic
Actions

Authored by kp on May 10 2022, 8:53 AM.

Details

Reviewers

None

Group Reviewers

network
pfsense

Commits

rG37c452292132: pf: also apply dummynet to route-to/dup-to packets

Summary

If packets are processed by a route-to/dup-to/reply-to rule (i.e. they
pass through pf_route(6)) dummynet was not applied to them.
This is because pf_route(6) passes packets directly to ifp->if_output(),
so the dummynet functions were never called.

Factor out the dummynet code and call dummynet prior to
ifp->if_output(). This has a secondary benefit of reducing some code
duplication between the IPv4 and IPv6 paths.

Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 45507
Build 42395: arc lint + arc unit

Event Timeline

kp created this revision.May 10 2022, 8:53 AM

Herald added subscribers: glebius, melifaro, farrokhi, imp. · View Herald TranscriptMay 10 2022, 8:53 AM

kp requested review of this revision.May 10 2022, 8:53 AM

kp added a child revision: D35159: pf: tag dummynet'd route-to packets with their real destination.

Harbormaster completed remote builds in B45507: Diff 105802.May 10 2022, 8:53 AM

The new function is bool, to tell if the packet was consumed. Let's call this modern FreeBSD style. You also pass a pointer to pointer, so function can NULL-ify to express same fact of consumed mbuf. This matches classic OpenBSD/pf style. In one case you use FreeBSD style and in two cases OpenBSD.

IMHO, the new function should be used consistently using either return value or the pointer. Personally I prefer FreeBSD style over OpenBSD/pf.

In D35158#797201, @glebius wrote:

The new function is bool, to tell if the packet was consumed. Let's call this modern FreeBSD style. You also pass a pointer to pointer, so function can NULL-ify to express same fact of consumed mbuf. This matches classic OpenBSD/pf style. In one case you use FreeBSD style and in two cases OpenBSD.

IMHO, the new function should be used consistently using either return value or the pointer. Personally I prefer FreeBSD style over OpenBSD/pf.

That's not quite right. The return value indicates an error. For example when we fail to allocate the tag, or if we're configured to shape the packet (i.e. dnpipe/dnrpipe != 0) but dummynet is not loaded.

We can return 'true' and yet not have eaten the mbuf. I'll change the return type to an int (and return ENOMEM on error) to make it a bit clearer.

Change return type to int, to clarify that this is an error return.

Harbormaster completed remote builds in B45525: Diff 105853.May 11 2022, 8:48 AM

This revision was not accepted when it landed; it landed in state Needs Review.May 12 2022, 7:58 PM

Closed by commit rG37c452292132: pf: also apply dummynet to route-to/dup-to packets (authored by kp). · Explain Why

This revision was automatically updated to reflect the committed changes.

kp added a commit: rG37c452292132: pf: also apply dummynet to route-to/dup-to packets.