Page MenuHomeFreeBSD

ifconfig: redo fix vlan/vlanproto reconfiguration
ClosedPublic

Authored by zlei on May 21 2024, 10:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 3:03 AM
Unknown Object (File)
Fri, Nov 1, 3:02 AM
Unknown Object (File)
Fri, Nov 1, 3:02 AM
Unknown Object (File)
Fri, Nov 1, 2:40 AM
Unknown Object (File)
Sep 30 2024, 3:26 PM
Unknown Object (File)
Sep 30 2024, 3:24 PM
Unknown Object (File)
Sep 30 2024, 1:27 PM
Unknown Object (File)
Sep 30 2024, 12:06 AM

Details

Summary

In case the if_vlan(4) interface has not been configured correctly, i.e. a bare interface without physical interface associated with it, retrieve the current settings and unconditionally overwrite params will lose vlandev settings. That will lead false report 'both vlan and vlandev must be specified'.

PR: 279181
Fixes: b82b8055ad44 ifconfig: fix vlan/vlanproto reconfiguration
MFC after: 3 days /* maybe should fast so catch up 14.1 ? */

Test Plan
# ifconfig vlan0 create
# ifconfig vlan0 vlandev cxl0 vlan 100 vlanproto 802.1ad
# ifconfig vlan0 | grep vlanproto
	vlan: 100 vlanproto: 802.1ad vlanpcp: 0 parent interface: cxl0
# ifconfig vlan0 vlan 101
# ifconfig vlan0 | grep vlanproto
	vlan: 101 vlanproto: 802.1ad vlanpcp: 0 parent interface: cxl0
# ifconfig vlan0 vlanproto 802.1q
# ifconfig vlan0 | grep vlanproto
	vlan: 101 vlanproto: 802.1q vlanpcp: 0 parent interface: cxl0

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

zlei requested review of this revision.May 21 2024, 10:19 AM

I guess I need a better Title for this CR , I dislike the redo ...

diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 675ed0090e8c..4d5d70410898 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -37,7 +37,7 @@ basic_body()
        # And change back
        # Test changing the vlan ID
        atf_check -s exit:0 \
-           jexec singsing ifconfig ${vlan1} vlandev ${epair_vlan}b vlan 42
+           jexec singsing ifconfig ${vlan1} vlan 42 vlandev ${epair_vlan}b
        atf_check -s exit:0 -o ignore jexec singsing ping -c 1 10.0.0.1
 }

is probably enough for a test case.

The patch looks reasonable, but I'd like to test it in combination with my pending vlan fix too.

I've fixed the other vlan issue in https://reviews.freebsd.org/D45285.

Please include the test change I mentioned above, then go ahead and commit this.

This revision is now accepted and ready to land.May 21 2024, 12:29 PM
In D45283#1033101, @kp wrote:
diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 675ed0090e8c..4d5d70410898 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -37,7 +37,7 @@ basic_body()
        # And change back
        # Test changing the vlan ID
        atf_check -s exit:0 \
-           jexec singsing ifconfig ${vlan1} vlandev ${epair_vlan}b vlan 42
+           jexec singsing ifconfig ${vlan1} vlan 42 vlandev ${epair_vlan}b
        atf_check -s exit:0 -o ignore jexec singsing ping -c 1 10.0.0.1
 }

is probably enough for a test case.

No, that is not enough. The parent (physical) dev should be disassociated to match the bug.

diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
index 675ed0090e8c..458e3cc36bc6 100755
--- a/tests/sys/net/if_vlan.sh
+++ b/tests/sys/net/if_vlan.sh
@@ -22,8 +22,12 @@ basic_body()
        jexec alcatraz ifconfig ${epair_vlan}a up
        jexec alcatraz ifconfig ${vlan0} 10.0.0.1/24 up
 
-       vlan1=$(jexec singsing ifconfig vlan create vlandev ${epair_vlan}b \
-               vlan 42)
+       vlan1=$(jexec singsing ifconfig vlan create)
+
+       # Test associating the physical interface
+       atf_check -s exit:0 \
+           jexec singsing ifconfig ${vlan1} vlandev ${epair_vlan}b vlan 42
+
        jexec singsing ifconfig ${epair_vlan}b up
        jexec singsing ifconfig ${vlan1} 10.0.0.2/24 up
 
@@ -37,7 +41,7 @@ basic_body()
        # And change back
        # Test changing the vlan ID
        atf_check -s exit:0 \
-           jexec singsing ifconfig ${vlan1} vlandev ${epair_vlan}b vlan 42
+           jexec singsing ifconfig ${vlan1} vlan 42 vlandev ${epair_vlan}b
        atf_check -s exit:0 -o ignore jexec singsing ping -c 1 10.0.0.1
 }

@kp I'm going to commit this if no objections.

The patch looks reasonable, but I'd like to test it in combination with my pending vlan fix too.

@kp I'm going to commit this if no objections.

No objection. Go ahead and commit.