OPNsense Forum
English Forums => Development and Code Review => Topic started by: marjohn56 on December 24, 2017, 01:25:12 am
-
My first post here as I've just moved from the dark side.
I have a couple of newbie development questions.
I have had several commits for various additions and changes accepted on the dark side, and I want to bring those same features to opnsense. Now I have a few testers who are also making the move and in the past I would issue a commit and they would use the system patches feature to apply and test. It seems that to do the same with opnsense it needs to be done from the shell, is that correct?
I also have questions on the binaries dhcp6c and dhclient, I assume dhclient is in use on opnsense - I haven't checked, the changes I have issued commits for these upstream, and they are needed for orange France, they have been hanging around for many months, obviously french users are not that important to the dark side. I'll post more on these once I have got my head around the way opnsense does things.
-
Hi there and weclome!
I have had several commits for various additions and changes accepted on the dark side, and I want to bring those same features to opnsense. Now I have a few testers who are also making the move and in the past I would issue a commit and they would use the system patches feature to apply and test. It seems that to do the same with opnsense it needs to be done from the shell, is that correct?
That's correct, there is opnsense-patch(8) and it also has a man page. In a nutshell it's using GitHub commit diffs identified by hash... you can also let others test from other accounts or repo names.
We once wanted to add an API call, but the effects of patching can be too difficult to revert from the GUI so we did not do this in the end.
I also have questions on the binaries dhcp6c and dhclient, I assume dhclient is in use on opnsense - I haven't checked, the changes I have issued commits for these upstream, and they are needed for orange France, they have been hanging around for many months, obviously french users are not that important to the dark side. I'll post more on these once I have got my head around the way opnsense does things.
Just push the patches our way and we will review, include and test together. We also have another dhclient custom patch awaiting FreeBSD inclusion:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217978
It has been included in the src.git and subsequently shipped...
dhcp6c is whatever FreeBSD has at the moment in ports. But a FreeBSD maintainer is writing this project so upstream inclusion or staging in our ports tree is possible.
If you have more questions just let me know. :)
Cheers,
Franco
-
Thanks Franco, it's nice to get a response that is positive and informative.
You can take a look at the dhcp6c as it's sitting at https://github.com/hrs-allbsd/wide-dhcpv6/pulls, mine is the PR for adding RAW options. I cannot take all the credit for it as the hard work was done by a french guy who does not want any credit, I just took his additions and created the PR. It works and is in use by many range FR users. Some of my other dhcp6c changes made it through a while back.
The dhclient changes are, as you may be aware fix a reversion that broke the client for orange fr users, is it the same issue that you have a fix for.. forget that I've looked and it's not. Ok, I'll try and find the commit again.
-
Yep I'm one of the "testers" and I can say its great to have escaped
-
Thanks Franco, it's nice to get a response that is positive and informative.
No thanks required as this is how it should be. :)
You can take a look at the dhcp6c as it's sitting at https://github.com/hrs-allbsd/wide-dhcpv6/pulls, mine is the PR for adding RAW options. I cannot take all the credit for it as the hard work was done by a french guy who does not want any credit, I just took his additions and created the PR. It works and is in use by many range FR users. Some of my other dhcp6c changes made it through a while back.
Ah, in that case thank you for you work that has reached us already.
I will say the raw support seems like a shortcut and I'm not opposed to if it addresses a need for users, but for upstream inclusion it could be hard to review, dives in deep and may delay inclusion. Maybe we can break it down to more user-oriented features?
The dhclient changes are, as you may be aware fix a reversion that broke the client for orange fr users, is it the same issue that you have a fix for.. forget that I've looked and it's not. Ok, I'll try and find the commit again.
I'm not sure which issue you refer to, FreeBSD or pfSense? We use FreeBSD 11.0 and soon 11.1 and the only patch we have is this the bug report I sent earlier which correspond to this commit:
https://github.com/opnsense/src/commit/14580e78e
If you could point me towards a commit that breaks or repairs a use case I can take a closer look if necessary.
Yep I'm one of the "testers" and I can say its great to have escaped
Welcome to you, too. :)
Cheers,
Franco
-
Nivek can tell you more about the dhclient regression, I believe it was fixed and worked in FreeBSD 10 and was broken again in 11.*
Dhcp6c code is particularly evil around the options area - hence the raw options as there will always be one that has not been added, raw options it least allows any option to be specified.
-
I see. If there is an issue with FreeBSD 11.1 we should act fast so that we can offer a solution when 18.1 comes out correctly backed into the images.
As for dhcp6c, you are probably right. The question then is how much would OPNsense depend on it... we'd have to add options which use that raw input options as the second integration step, right?
Cheers,
Franco
-
I don't believe anything needs to be done to opnsense to use then raw options, the ability to send options is already there. Nivek1612 will be testing it shortly, he has systems in France and the UK so he is very useful for testing.:)
Dhclient was a regression with option77.
Here's a link to my PR.
https://github.com/pfsense/FreeBSD-src/pull/8
-
I'm using standard pfSense 2.4.3 (sorry) on an AP2U connecting to Orange FTTP in FRANCE (with one additonal patch Marjohn provided for dhcp6c VLAN priority tagging - which is waiting to be committed) and then the two modified clients dhclient and dhcp6c.
In that form I'm able to fully utilise my 500GB/250GB FTTP session without issues
Its been stable for 3 months
-
I will be setting up a test system in the next day or so with OPNSense 17.7.11
I will then replace the stock dhclient and dhcp6c with those patched by Marjohn and wireshark trace the dhcp and dhcp6c request to ensure the correct options are being passed and report back here on my findings
My intitial look at the code suggest it should work
-
FWIW, you can reuse the other binaries until we fix them here, they should be compatible so care should only be taken during updates where they could be overwritten, which could be "fixed" by using the rc.syshook stuff moving the other binaries back into place. :)
https://docs.opnsense.org/development/backend/autorun.html
Cheers,
Franco
-
Thanks Franco
-
Have to say I'm impressed with the pro-active responses.
My next job is to store the DUID in the config, and this is a requirement we found was needed for users of Sky in the UK and one or two other ISPs. It should really only apply to users using RAM disks, but obviously the DUID can also be lost when upgrading or re-installation. Once it is stored in the config then the problem does not exist.
-
I can confirm that the modified dhclient and dhcp6c (provided by Marjohn) work as expected and pass all the required parameters
With the addition of a new VLAN priority tagging patch that Marjohn created for pfSense it will all most be there
However it appears OPNsense is suffering the same issue has described here
https://redmine.pfsense.org/issues/7973#change-34905
In that priority tags are not being honoured
-
I'm on it... just testing something now, but I may not be able to finish it until Thursday,
-
OK, looks like the same issue as was in p*****e, missing net.link.vlan.mtag_pcp" => "1 for one, and the need to add the set prio on the dhcpv6 packets.
I've proved the second part and the rules are now showing the set prio info, but i need to do the net.link bit, that will be tomorrow.
-
REDACTED ;D
-
wrong thread. : :P
-
Lol, I was searching for "dark". Thanks. I'll fix.