Align refcounting in multicast code between IPv4 and IPv6
ClosedPublic
Actions

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

Details

Reviewers

ae
rgrimes

Group Reviewers

network

Commits

rS355576: MFC r355322:
rS355322: Use refcount from "in_joingroup_locked()" when joining multicast

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 - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• hselasky created this revision.Nov 28 2019, 6:29 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 28 2019, 6:29 PM

Harbormaster completed remote builds in B27838: Diff 65016.Nov 28 2019, 6:29 PM

afedorov added a subscriber: afedorov.Nov 29 2019, 6:40 AM

evgueni.gavrilov_itglobal.com added a subscriber: evgueni.gavrilov_itglobal.com.Nov 29 2019, 6:47 AM

• hselasky added a reviewer: network.Nov 30 2019, 4:48 AM

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.Dec 3 2019, 7:38 AM

• hselasky added inline comments.Dec 3 2019, 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.