Page MenuHomeFreeBSD

if_clone: Allow maxunit to be zero
ClosedPublic

Authored by zlei on Thu, Jun 27, 3:55 PM.
Tags
None
Referenced Files
F87519576: D45757.id140324.diff
Thu, Jul 4, 8:57 AM
F87518583: D45757.id140307.diff
Thu, Jul 4, 8:36 AM
F87468450: D45757.vs140415.id140505.diff
Wed, Jul 3, 1:21 PM
F87468316: D45757.diff
Wed, Jul 3, 1:20 PM
Unknown Object (File)
Tue, Jul 2, 7:31 AM
Unknown Object (File)
Sun, Jun 30, 5:36 PM
Unknown Object (File)
Sat, Jun 29, 7:43 AM
Unknown Object (File)
Fri, Jun 28, 11:16 PM
Subscribers

Details

Summary

Some drivers, e.g. if_enc(4), only allow one instance to be created, but
the KPI ifc_attach_cloner() treat zero as not limited, aka IF_MAXUNIT .

Introduce a new flag IFC_F_LIMITUNIT to indicate that the provided
maxunit is limited and should be respected.

Consumers should use the new flag if there is an intended limit.

MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zlei requested review of this revision.Thu, Jun 27, 3:55 PM

Shouldn't that be done to if_clone_addreq_v2, too?

sys/net/if_clone.c
533–535

Do we really need the old behavior at all? There are no consumers in tree that initialize maxunit.
So could be:

maxunit = (req->flags & IFC_F_LIMITUNIT) ? req->maxunit : IF_MAXUNIT;

Added asserting on the compatibility of layout of struct if_clone_addreq and struct if_clone_addreq_v2 .

Shouldn't that be done to if_clone_addreq_v2, too?

The layout of struct if_clone_addreq_v2 is compatible with that of struct if_clone_addreq . i.e., maxunit of if_clone_addreq_v2 is the same with if_clone_addreq . I added a compile assert to reveal that.

sys/net/if_clone.c
533–535

Do we really need the old behavior at all? There are no consumers in tree that initialize maxunit.
So could be:

maxunit = (req->flags & IFC_F_LIMITUNIT) ? req->maxunit : IF_MAXUNIT;

That is exactly same with my first revision. Well later I decided to MFC this change and I worried 3rd party drivers may have already used the new KPI so I restored the old behavior.

IMO the breakage should be trivial to fix. But it seems neither RELNOTES nor UPDATING is the right place to warn developers. Any suggestions ?

zlei edited the summary of this revision. (Show Details)

Removed compatibility for old behavior.

This revision is now accepted and ready to land.Mon, Jul 1, 10:48 PM

Sorry for delay!

Do not worry. I'm just a little eager to have this change in ;)

This revision was automatically updated to reflect the committed changes.