Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ipv6 AF_INET6 support #6

Closed
lamontj opened this issue Sep 9, 2016 · 19 comments
Closed

ipv6 AF_INET6 support #6

lamontj opened this issue Sep 9, 2016 · 19 comments
Milestone

Comments

@lamontj
Copy link
Contributor

lamontj commented Sep 9, 2016

1.4.11 lacks any ipv6 support. 1.5.3 (current upstream from 1 Aug 2016) also lacks ipv6 support.

Note that both ipmitool (since 2014 or so) and openipmi (since 2003-11-11) support IPv6, according to their changelogs. I suspect that the openipmi stuff is either vendor-specific, or simply that it started using AF_INET6 sockets...

At a minimum, the things we've run into:

  1. Cannot specify an ipv6 address for the bmc address, to any of the tools.
  2. bmc-config --checkout doesn't dump ipv6 address information.

Happy to test changes -- generally using HP Proliant's with iLo 4 or so.

@lamontj
Copy link
Contributor Author

lamontj commented Sep 9, 2016

@chu11
Copy link
Owner

chu11 commented Sep 19, 2016

Thanks for the report. There appears to be two issues.

  1. IPv6 support in ipmi-config and possibly spillover effect into other tools (TBD)
  2. AF_INET6 support, which I've never tested at all. I'm 99% sure this was not looked into when FreeIPMI was first started in 2003.

Don't know when I can get to this. Major changes in FreeIPMI are usually forced upon me by my organization needs and this one is decently major. But perhaps I can slowly try to add piecemeal (minimally part 2 above).

@lamontj
Copy link
Contributor Author

lamontj commented Nov 21, 2016

Here is an initial run at adding IPv6 support to ipmipower. Discovery of the ipv6 address is a much different can of worms. (FWIW, hponcfg -all -w foo.xml DOES dump it, so it's definitely available somehow.)

I have one issue with this patch, that I would really like to see dealt with: In every other tool out there, ipv6 literal addresses (esp where port numbers are present), are either "IP:v6::addr:required_port" or "[IP:v6::addr]:optional_port". ipmipower has already assigned syntactical significance to [], which I believe could be extended to allow ipv6 literals to use [] as well. For the moment and for my testing, I just went with {} instead of [], and have verified that ipmipower works when talking to a Proliant DL380 G7 with current firmware. (1.80 firmware does not listen on the IPv6 impi port, 1.88 does.)
ipv6-power.patch.txt

I'd like to get something landed in the upstream this week, if at all possible. In the best case, with a final decision on how to add ipv6 literals to the syntax.

@chu11
Copy link
Owner

chu11 commented Nov 21, 2016

Hi Lamont, thanks for taking an initial crack at this. Need to look into your patch more, but in the mean time could you rebase against master and send a github PR? We can iterate easier that way. Thanks.

@chu11
Copy link
Owner

chu11 commented Nov 22, 2016

First pass (without trying it, just skimming) looks pretty good. I'm curious what other tools support the "[IP:v6::addr]:optional_port" format you mentioned above. I want to ask some of the people here
about their opinion on this, b/c it's something that could affect many tools we use internally.

Is this something you need on the soon-ish side in ipmipower specifically? Thus your hope for this to move fast? Or did you view this generically speaking as the beginning of a patch for something
bigger in FreeIPMI? There are some big internal changes for FreeIPMI in the future 1.6 release. So IPv6 would be perfect to try and aim for that release.

@lamontj
Copy link
Contributor Author

lamontj commented Nov 22, 2016

If we settle on the syntax for ipv6 literals, and know that they're coming in 1.6, then I have no issue with us maintaining the resulting fork for 1.4 and 1.5. If we go the route of expanding the [] syntax, I'd really love for someone more familiar with that parser to do the change.

@chu11
Copy link
Owner

chu11 commented Nov 22, 2016

Yeah, IPv6 wouldn't go into the 1.4 or 1.5 line. It'd definitely be for a future 1.6 line (master branch). If your concern was mostly over conflicts in maintenance branches, don't worry about it for FreeIPMI 1.4/1.5 line. That should hopefully allow you to get your fix into ubuntu archive on schedule.

@chu11 chu11 added this to the 1.6 milestone Nov 22, 2016
@chu11 chu11 changed the title lacks ipv6 support ipv6 AF_INET6 support Nov 23, 2016
@chu11
Copy link
Owner

chu11 commented Nov 23, 2016

I'm splitting this issue into two, one is AF_INET6 support and IPv6 ipmi-config will be another issue.

@chu11
Copy link
Owner

chu11 commented Dec 6, 2016

Anyone wandering to this issue, you may see my (likely slow) progress on branch "ipv6" here on github.

@chu11
Copy link
Owner

chu11 commented Jan 5, 2017

@lamontj I think I've completed AF_INET6 support in the previously mentioned "ipv6" branch. If you have a chance, could you check it out? See if there's anything obvious missing before I merge into master. Thanks

@lamontj
Copy link
Contributor Author

lamontj commented Jan 6, 2017

I should be able to work through this early next week.

@lamontj
Copy link
Contributor Author

lamontj commented Jan 9, 2017

With c095144, there's only one small issue:
sudo ipmipower --driver-type=LAN_2_0 -W opensesspriv -h '[2001:db8:3::f]:623' -u $USER -p $PASS --stat
[2001:db8:3::f]: on
sudo ipmipower --driver-type=LAN_2_0 -W opensesspriv -h '[2001:db8:3::f]' -u $USER -p $PASS --stat
[2001:db8:3::f]: invalid hostname
Beyond that, nothing seems amiss.

@chu11
Copy link
Owner

chu11 commented Jan 9, 2017

At the moment the code does consider [2001:db8:3::f] an invalid hostname, should it not? I figure the brackets should only be there if the user will specify a port.

@lamontj
Copy link
Contributor Author

lamontj commented Jan 9, 2017

The issue is that 2001:db8:3::100 is ambiguous as to whether it means port 100 on 2001:db8:3::, or the default port on 2001:db8:3:100. Apache and others require the brackets to always surround IPv6 literals, whether a port is present or not.

@chu11
Copy link
Owner

chu11 commented Jan 9, 2017

Ahhh, I didn't see the double colon. That's something I'll have to add as another special case. Hmmm, perhaps will require another layer of special casing as most of the code is now:

if host_with_port
   deal w/ host and port
else
  host is whatever was passed in

so gotta add an else-if or maybe a wrapper function that handles all of it.

@chu11
Copy link
Owner

chu11 commented Jan 10, 2017

@lamontj The fix was way easier than I thought it'd be, pumped it out this afternoon. I non-fast-forwarded the branch so you may want to recreate the branch from scratch.

@chu11
Copy link
Owner

chu11 commented Jan 13, 2017

@lamontj How's this look? I'm thinking its good to merge into master now.

@lamontj
Copy link
Contributor Author

lamontj commented Jan 14, 2017

Looks good to me:
ipmipower -D LAN_2_0 -h '[fd99::f]' -u $U -p $P --stat
fd99::f: on
ipmipower -D LAN_2_0 -h '[fd99::f]:623' -u $U -p $P --stat
fd99::f: on
ipmipower -D LAN_2_0 -h '10.246.88.15' -u $U -p $P --stat
10.246.88.15: on
ipmipower -D LAN_2_0 -h '10.246.88.15:623' -u $U -p $P --stat
10.246.88.15: on

@chu11
Copy link
Owner

chu11 commented Jan 14, 2017

Awesome thanks. It's merged upstream now.

@chu11 chu11 closed this as completed Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants