Page MenuHomeFreeBSD

Align refcounting in multicast code between IPv4 and IPv6
ClosedPublic

Authored by hselasky on Thu, Nov 28, 6:29 PM.

Details

Summary

Use refcount from "in_joingroup_locked()" when joining multicast groups. Do not acquire additional references.
This makes the IPv4 IGMP code in line with the IPv6 MLD code.

Background:
The IPv4 multicast code puts an extra reference on the in_multi struct when joining groups.
This becomes visible when using daemons like igmpproxy from ports, that multicast entries
do not disappear from the output of ifmcstat(8) when multicast streams are disconnected.

This fixes a regression issue after r349762.

Tested by: Guido van Rooij <guido@gvr.org>
MFC after: 1 week
Sponsored by: Mellanox Technologies

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

hselasky created this revision.Thu, Nov 28, 6:29 PM
rgrimes accepted this revision.Tue, Dec 3, 7:38 AM
rgrimes added a subscriber: rgrimes.

Thanks for quickly addressing this. Just a code comment I noticed, and am curious if I missed something that would not allow the removal of a duplicate conditional

sys/netinet/in_mcast.c
2242 ↗(On Diff #65016)

Why restest is_new? Cant this line just be moved up to the last line of the prior if (is_new) right where the lock aquire was done before?

This revision is now accepted and ready to land.Tue, Dec 3, 7:38 AM
hselasky added inline comments.Tue, Dec 3, 8:04 AM
sys/netinet/in_mcast.c
2242 ↗(On Diff #65016)

Then we would need to change the error out logic too, I.E. remove the entry upon failure. I think it is best to not insert the entry before everything is clear.

hselasky added inline comments.Tue, Dec 3, 8:12 AM
sys/netinet/in_mcast.c
2242 ↗(On Diff #65016)

I see your point. Let me re-order.

hselasky marked an inline comment as done.Tue, Dec 3, 8:12 AM
hselasky updated this revision to Diff 65153.Tue, Dec 3, 8:17 AM

Implement suggestion from @rgrimes .

This revision now requires review to proceed.Tue, Dec 3, 8:17 AM
rgrimes accepted this revision.Tue, Dec 3, 8:30 AM

Thanks

This revision is now accepted and ready to land.Tue, Dec 3, 8:30 AM