Assign Interface doesn't require Confirmation

eri--

Normally that is not true.
If you do not hit save or anything like that nothing will break.

If you forget to assign the correct interface than surely you have issues.
Though more details are needed on how to reproduce this if its really the case.

doktornotor

@ermal:

Though more details are needed on how to reproduce this if its really the case.

Well, this is perfectly reproducible for anyone once you have only one unasassigned iface left… no matter whether it's OPT, OpenVPN or whatever else.

jimp

Yes, when you click + it will latch onto the next port, but until you enable that interface, it doesn't do anything with it.

I haven't seen it break OpenVPN before until I actually went in and fiddled with the interface, but it's been a while since I tried. It's not out of the question that it could interfere in some way.

Changing the values in the drop-down does require a 'save' but clicking + does not.

phil.davis

interfaces_assign.php lines 361 to 366:

	/* XXX: Do not remove this. */
mwexec("/bin/rm -f /tmp/config.cache");

write_config();

$savemsg = gettext("Interface has been added.");

When the add button has been pressed it makes a new entry in the table and gives it a default port of the first port it finds that is not used yet - good. Then it does this business of removing config.cache and doing a write_config(). The user never gets a chance to confirm this stuff.
For an easy example, I have LAN and WAN on an Alix, my config has no reference to OPT1. I press the "+" to add an interface. OPT1 appears on the GUI table, with vr2 - good. But if I search my config now I find references to OPT1. And if I open another GUI browser tab, the Interfaces menu dropdown has OPT1 in it already. But the Interfaces Assign GUI page has a "Save" button there, looking like it hasn't implemented anything yet.
A more complex example would have it picking an existing OpenVPN server instance port, and the unconfirmed write_config() might impact the functioning of that OpenVPN server until the user picks the network port that they really want from the dropdown and presses save.
Is there a reason this write_config() is required at this point?
Can it simply be removed from the code? and the stuff that happens in response to the Save button will do the implementing?
"/* XXX: Do not remove this. */" looks like someone had a reason for this being done like this, but the reason is not in the code comments.

jimp

If you don't write the change to the config, it wouldn't be saved.

The problem is that the row isn't added by javascript, it's added by the GET request for ?add. What people are expecting is the row to just pop up like it does with other rowhelpers when in fact it does a separate http request which adds it and then allows changes.

So it would be non-trivial to change. Either way that write_config() is required and can't be removed. That does not "apply" – it just saves the changes into config.xml, which is required the way the page currently operates.

cthomas

It may not 'apply', but when I tried to assign an interface on my firewall at the data center, and the ovpn3 interface was auto-populated, it immediately broke the ovpn3 tunnel. I had to disable/enable the OpenVPN connection in order to restore connectivity. This was on 2.0.3

I'm in the process of building out a new set of pfsense firewalls running 2.1 RC0, and while attempting to assign an interface, the lan_vip1 (CARP) interface was automatically populated, which broke lan_vip1 until I edit/save the properties of the CARP Virtual Interface. I noticed it immediately when it happened because I was running a continuous ping to the vip and I saw it drop as soon as I hit the button.

eri--

Which button?
No events happen during that new entry. If you hit save surely you have to leave with whatever choice you made!

Klaws

The (+) button (aka "add" button). It creates a new Interface with a default name (e.g. OPT2), assigns it to some available Network Port, and saves the config. No need to hit "Save". It's stateless.

Next, the user may change the automatically assigned Network Port to the correct one (and then hits "Save").

Between these two steps, the Interface is configured with the automatically chosen NetworkPort. Just tried it. In my case, the automatically chosen Network Port belongen to a VPN which was up and running at this time. And the hapless user's VPN connection got kicked. Hehe, (snort), who cares about users, har har, he probably deserved this anyway, roflmao! ;-)

However. I then contemplated about the possibility that I had been remotely confguring pfSense via the VPN connection. I then suddenly began to dislike the idea of random VPN users getting kicked out.

Proposed solution: add an entry field and a drop-down-box in the table row of the (+)/add button. This would allow the user to pre-select the correct values.

cthomas

Sounds like we're all in agreement that the workflow for assigning interfaces should be modified a bit, is there anything else that I can, or need to do to help get this in the development pipeline?

Klaws

@cthomas:

is there anything else that I can, or need to do to help get this in the development pipeline?

Open a bug report or CR on http://redmine.pfsense.org, if you haven't already done so?