Moved from the dark side.

Started by marjohn56, December 24, 2017, 01:25:12 AM

Previous topic - Next topic
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.
OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member

Hi there and weclome!

Quote from: marjohn56 on December 24, 2017, 01:25:12 AM
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.

Quote from: marjohn56 on December 24, 2017, 01:25:12 AM
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

December 24, 2017, 12:51:59 PM #2 Last Edit: December 24, 2017, 01:45:23 PM by marjohn56
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.

OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member

Yep I'm one of the "testers" and I can say its great to have escaped
OPNsense 24.7.* on Qotom i5-5250U with AAISP FTTP 900/120
OPNsense 24.7.* on Qotom i7-4500U with Orange FR FTTP 1000/400

Team Rebellion Member
One of Marjohns TESTERS :-)

Quote from: marjohn56 on December 24, 2017, 12:51:59 PM
Thanks Franco, it's nice to get a response that is positive and informative.

No thanks required as this is how it should be. :)

Quote from: marjohn56 on December 24, 2017, 12:51:59 PM
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?

Quote from: marjohn56 on December 24, 2017, 12:51:59 PM
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.

Quote from: nivek1612 on December 24, 2017, 02:01:44 PM
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.
OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member

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
OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member

December 24, 2017, 03:18:34 PM #8 Last Edit: December 24, 2017, 04:07:37 PM by nivek1612
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
OPNsense 24.7.* on Qotom i5-5250U with AAISP FTTP 900/120
OPNsense 24.7.* on Qotom i7-4500U with Orange FR FTTP 1000/400

Team Rebellion Member
One of Marjohns TESTERS :-)

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
OPNsense 24.7.* on Qotom i5-5250U with AAISP FTTP 900/120
OPNsense 24.7.* on Qotom i7-4500U with Orange FR FTTP 1000/400

Team Rebellion Member
One of Marjohns TESTERS :-)

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
OPNsense 24.7.* on Qotom i5-5250U with AAISP FTTP 900/120
OPNsense 24.7.* on Qotom i7-4500U with Orange FR FTTP 1000/400

Team Rebellion Member
One of Marjohns TESTERS :-)

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.
OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member

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
OPNsense 24.7.* on Qotom i5-5250U with AAISP FTTP 900/120
OPNsense 24.7.* on Qotom i7-4500U with Orange FR FTTP 1000/400

Team Rebellion Member
One of Marjohns TESTERS :-)

I'm on it... just testing something now, but I may not be able to finish it until Thursday,
OPNsense 25.7a - Qotom Q355G4 - ISP - Squirrel 1Gbps.

Team Rebellion Member