troglobit / pimd

PIM-SM/SSM multicast routing for UNIX and Linux
http://troglobit.com/projects/pimd/
BSD 3-Clause "New" or "Revised" License
200 stars 90 forks source link

PIM-SSM support and some misc changes #36

Closed idismmxiv closed 9 years ago

idismmxiv commented 9 years ago

Initial PIM-SSM support implementation and several other misc changes. All changes are not fully RFC compliant as for example Hello Generation ID is only sent, but not properly handled when received one. But here they are for your comments and review.

troglobit commented 9 years ago

This is a huge pull request and I'm very grateful for your submission to pimd! The commits are logical and isolated to relevant changes and follow the coding style of pimd. :+1:

It looks like I can pull this right in, but I'd very much like to give you the proper credit for this herculean effort, both in the AUTHORS file and in the commits ... is your name really just 'Idis'? I can fully understand if you want to remain anonymous, but you really do deserve credit for this work!

idismmxiv commented 9 years ago

Its good to hear that the contribution is acceptable. There is a small group of people doing and reviewing changes, but main credit goes to Markus Veranen (markus.veranen@gmail.com).

troglobit commented 9 years ago

Markus it is then.

I'll probably do a 2.2.1 with the last fix to #22 (issue #37), before I merge this massive changeset as 2.3.0 and release.

Cheers!

troglobit commented 9 years ago

I've started merging the patches, with only slight cleanup in syntax. Apart from the first patch, which adds the new configure script switches, that's been heavily refactored. Take a look at 403ddce and see if you approve. Hopefully I haven't introduced any regressions.

All patches are merged to the pim-ssm branch, but I haven't yet gone through them all in full detail yet. Hope to get some more time for this later this week.

idismmxiv commented 9 years ago

The first patch looks ok to me.

troglobit commented 9 years ago

Thx!

idismmxiv commented 9 years ago

Found out that IGMPv3 general query misses some mandatory values. So added qrv and qqic values to be able to follow RFC.

To get qqic correctly generated I took one function from quagga pim daemon. I am not sure if that is ok due to bsd license. If it is not OK to use even single function, then that must be rewritten.

troglobit commented 9 years ago

@idismmxiv Good catch, the qrv/qqic bits are important!

Unfortunately even minor code sharing like that would "pollute" the BSD code base and likely make it GPL.

idismmxiv commented 9 years ago

Added a new implementation for the IGMPv3 floating point calculation. I've ran it through some tests to verify that the QQIC values are calculated correctly. Also, tested running it with IGMPv2 and IGMPv3 (with different QQI values) and it seemed to work OK.

troglobit commented 9 years ago

Great stuff, code looks very clean!

(I've gone on vacation now and am about to pick up some of the slack in my private projects, including pimd. Should be up to speed next week.)

Cheers!

troglobit commented 9 years ago

OK, just finished off the last bits of auditing and rebased it all on top of the latest master. Sorry for taking so long to prepare this!

If you have any time to spare, please have a look at https://github.com/troglobit/pimd/commits/pim-ssm before I merge it to master and release pimd 2.3.0. In particular, I squashed the last two commits (floating point).

Cheers!

troglobit commented 9 years ago

I've tested the pim-ssm branch now, works like a charm, even the IGMPv3 support! :smiley:

A couple of questions though:

  1. Shouldn't we make IGMPv3 the default, rather than the old IGMPv2? Possibly trigger down to IGMPv2 if we hear a IGMPv2 report/join, and reset to v3 on link down.
  2. I'd also like to take the opportunity of lowering the default IGMP query interval to, say 12 sec?

What do you think?

troglobit commented 9 years ago

Oh well, I'm going ahead with my changes to make IGMPv3 default and lowering the default query interval to 12 sec.

I'm merging to master and will start preparing the v2.3.0 release.

Just check back with me in the issue tracker if there are any problems, or my gmail address if there are any other comments.