Hi, Eelco

On Fri, Sep 19, 2025 at 11:23 PM Eelco Chaudron <[email protected]> wrote:
>
> On 11 Sep 2025, at 10:41, Changliang Wu wrote:
>
> Hi Changliang,
>
> Thanks for the patch, see some comments below.
>
> //Eelco
>
> I would be nice to get some description here.
Sure.

>
> > Signed-off-by: Changliang Wu <[email protected]>
> > ---
> >  tests/automake.mk  |   1 +
> >  tests/ovs-lldp.at  | 221 +++++++++++++++++++++++++++++++++++++++++++++
> >  tests/testsuite.at |   1 +
> >  3 files changed, 223 insertions(+)
> >  create mode 100644 tests/ovs-lldp.at
> >
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index 59f538761..a453a0c4b 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -68,6 +68,7 @@ TESTSUITE_AT = \
> >       tests/tunnel.at \
> >       tests/tunnel-push-pop.at \
> >       tests/tunnel-push-pop-ipv6.at \
> > +     tests/ovs-lldp.at \
> >       tests/ovs-router.at \
> >       tests/lockfile.at \
> >       tests/reconnect.at \
> > diff --git a/tests/ovs-lldp.at b/tests/ovs-lldp.at
> > new file mode 100644
> > index 000000000..81db5b517
> > --- /dev/null
> > +++ b/tests/ovs-lldp.at
> > @@ -0,0 +1,221 @@
> > +AT_BANNER([ovs-lldp])
> > +
> > +AT_SETUP([lldp - check lldp neighbor display])
> > +
> > +OVS_VSWITCHD_START([])
> > +
> > +add_of_ports br0 1
> > +AT_CHECK([
> > +    ovs-vsctl set Interface p1 type=dummy lldp:enable=true
> > +], [0])
> > +
> > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
> > +'0180c200000e38a91c18a12688cc020704aaaaaaaaaaaa0416054769676162697445746865726e6574312f302f313506020079081f4769676162697445746865726e6574312f302f313520496e746572666163650a0e46616b6553797374656d4e616d650c0e46616b6553797374656d446573630e0403140114100c0501c0a80001020000027b00fe060080c2010001fe070080c202020000fe100080c203000109564c414e2030303031fe060080c2060000fe090080c2070100000000fe0900120f01036c01001efe0c00120f0203010113000000fffe0600120f042800fe0900120f0301000000000000'])
> >
>
> Not sure how to make this nicer, but the hex string is hard to 
> modify/understand. Does anyone have any better ideas? Not sure what Scapy can 
> do these days.
There are a lot of hex packets in the existing cases. I will try to
use something like scapy to increase readability.

>
> > +AT_CHECK([ovs-appctl lldp/neighbor p1], [0], [dnl
> > +LLDP neighbor:
> > +Interface: p1
> > +  Chassis ID:         aa:aa:aa:aa:aa:aa
> > +  PortID:             GigabitEthernet1/0/15
> > +  TTL:                121
> > +  PortDescr:          GigabitEthernet1/0/15 Interface
> > +  SysName:            FakeSystemName
> > +  SysDescr:           FakeSystemDesc
> > +  Capability:         Bridge, on
> > +  Capability:         Router, on
> > +  MgmtIP:             192.168.0.1
> > +  MgmtIface:          635
> > +  MFS:                10240
> > +  PMD autoneg: supported: yes, enabled: yes
> > +    Adv:              10Base-T, HD: yes, FD: yes
> > +    Adv:              100Base-TX, HD: yes, FD: yes
> > +    Adv:              1000Base-T, HD: no, FD: yes
> > +    MAU oper type:    30
> > +  MDI Power: supported: yes, enabled: no, pair control: no
> > +  VLAN:               1, pvid: yes, VLAN 0001
> > +  PPVID:              supported: yes,enabled no
> > +])
> > +
> > +AT_CHECK([ovs-appctl --format json --pretty lldp/neighbor p1 ], [0], [dnl
> > +[{
> > +  "lldp": {
> > +    "interface": {
> > +      "p1": {
> > +        "chassis": {
> > +          "FakeSystemName": {
> > +            "capability": [
> > +              {
> > +                "enabled": true,
> > +                "type": "Bridge"},
> > +              {
> > +                "enabled": true,
> > +                "type": "Router"}],
> > +            "descr": "FakeSystemDesc",
> > +            "id": {
> > +              "type": "mac",
> > +              "value": "aa:aa:aa:aa:aa:aa"},
> > +            "mgmt-iface": [
> > +              635],
> > +            "mgmt-ip": [
> > +              "192.168.0.1"]}},
> > +        "port": {
> > +          "auto-negotiation": {
> > +            "current": 30,
> > +            "enabled": true,
> > +            "supported": true},
> > +          "desc": "GigabitEthernet1/0/15 Interface",
> > +          "id": {
> > +            "type": "ifname",
> > +            "value": "GigabitEthernet1/0/15"},
> > +          "mfs": 10240,
> > +          "power": {
> > +            "enabled": false,
> > +            "paircontrol": false,
> > +            "supported": true},
> > +          "ttl": 121},
> > +        "ppvid": {
> > +          "enabled": false,
> > +          "supported": true},
> > +        "vlan": {
> > +          "pvid": true,
> > +          "value": "VLAN 0001",
> > +          "vlan-id": 1}}}}}]
> > +])
> > +
> > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \
> > +'0180c200000e38a91c18a11c88cc020704aaaaaaaaaaaa0415054769676162697445746865726e6574312f302f3506020079081e4769676162697445746865726e6574312f302f3520496e746572666163650a0e46616b6553797374656d4e616d650c0e46616b6553797374656d446573630e0403140114100c0501c0a80001020000027b00fe060080c2010001fe070080c202020000fe100080c203000109564c414e2030303031fe060080c2060000fe090080c2070100000000fe0900120f01036c01001efe0c00120f0203010113000000fffe0600120f042800fe0900120f0301000000000000'])
> > +
> > +AT_CHECK([ovs-appctl lldp/neighbor p1], [0], [dnl
> > +LLDP neighbor:
> > +Interface: p1
> > +  Chassis ID:         aa:aa:aa:aa:aa:aa
> > +  PortID:             GigabitEthernet1/0/15
> > +  TTL:                121
> > +  PortDescr:          GigabitEthernet1/0/15 Interface
> > +  SysName:            FakeSystemName
> > +  SysDescr:           FakeSystemDesc
> > +  Capability:         Bridge, on
> > +  Capability:         Router, on
> > +  MgmtIP:             192.168.0.1
> > +  MgmtIface:          635
> > +  MFS:                10240
> > +  PMD autoneg: supported: yes, enabled: yes
> > +    Adv:              10Base-T, HD: yes, FD: yes
> > +    Adv:              100Base-TX, HD: yes, FD: yes
> > +    Adv:              1000Base-T, HD: no, FD: yes
> > +    MAU oper type:    30
> > +  MDI Power: supported: yes, enabled: no, pair control: no
> > +  VLAN:               1, pvid: yes, VLAN 0001
> > +  PPVID:              supported: yes,enabled no
> > +
> > +Interface: p1
> > +  Chassis ID:         aa:aa:aa:aa:aa:aa
> > +  PortID:             GigabitEthernet1/0/5
> > +  TTL:                121
> > +  PortDescr:          GigabitEthernet1/0/5 Interface
> > +  SysName:            FakeSystemName
> > +  SysDescr:           FakeSystemDesc
> > +  Capability:         Bridge, on
> > +  Capability:         Router, on
> > +  MgmtIP:             192.168.0.1
> > +  MgmtIface:          635
> > +  MFS:                10240
> > +  PMD autoneg: supported: yes, enabled: yes
> > +    Adv:              10Base-T, HD: yes, FD: yes
> > +    Adv:              100Base-TX, HD: yes, FD: yes
> > +    Adv:              1000Base-T, HD: no, FD: yes
> > +    MAU oper type:    30
> > +  MDI Power: supported: yes, enabled: no, pair control: no
> > +  VLAN:               1, pvid: yes, VLAN 0001
> > +  PPVID:              supported: yes,enabled no
> > +])
> > +
> > +AT_CHECK([ovs-appctl --format json --pretty lldp/neighbor p1 ], [0], [dnl
> > +[{
> > +  "lldp": {
> > +    "interface": [
> > +      {
> > +        "p1": {
> > +          "chassis": {
> > +            "FakeSystemName": {
> > +              "capability": [
> > +                {
> > +                  "enabled": true,
> > +                  "type": "Bridge"},
> > +                {
> > +                  "enabled": true,
> > +                  "type": "Router"}],
> > +              "descr": "FakeSystemDesc",
> > +              "id": {
> > +                "type": "mac",
> > +                "value": "aa:aa:aa:aa:aa:aa"},
> > +              "mgmt-iface": [
> > +                635],
> > +              "mgmt-ip": [
> > +                "192.168.0.1"]}},
> > +          "port": {
> > +            "auto-negotiation": {
> > +              "current": 30,
> > +              "enabled": true,
> > +              "supported": true},
> > +            "desc": "GigabitEthernet1/0/15 Interface",
> > +            "id": {
> > +              "type": "ifname",
> > +              "value": "GigabitEthernet1/0/15"},
> > +            "mfs": 10240,
> > +            "power": {
> > +              "enabled": false,
> > +              "paircontrol": false,
> > +              "supported": true},
> > +            "ttl": 121},
> > +          "ppvid": {
> > +            "enabled": false,
> > +            "supported": true},
> > +          "vlan": {
> > +            "pvid": true,
> > +            "value": "VLAN 0001",
> > +            "vlan-id": 1}}},
> > +      {
> > +        "p1": {
> > +          "chassis": {
> > +            "FakeSystemName": {
> > +              "capability": [
> > +                {
> > +                  "enabled": true,
> > +                  "type": "Bridge"},
> > +                {
> > +                  "enabled": true,
> > +                  "type": "Router"}],
> > +              "descr": "FakeSystemDesc",
> > +              "id": {
> > +                "type": "mac",
> > +                "value": "aa:aa:aa:aa:aa:aa"},
> > +              "mgmt-iface": [
> > +                635],
> > +              "mgmt-ip": [
> > +                "192.168.0.1"]}},
> > +          "port": {
> > +            "auto-negotiation": {
> > +              "current": 30,
> > +              "enabled": true,
> > +              "supported": true},
> > +            "desc": "GigabitEthernet1/0/5 Interface",
> > +            "id": {
> > +              "type": "ifname",
> > +              "value": "GigabitEthernet1/0/5"},
> > +            "mfs": 10240,
> > +            "power": {
> > +              "enabled": false,
> > +              "paircontrol": false,
> > +              "supported": true},
> > +            "ttl": 121},
> > +          "ppvid": {
> > +            "enabled": false,
> > +            "supported": true},
> > +          "vlan": {
> > +            "pvid": true,
> > +            "value": "VLAN 0001",
> > +            "vlan-id": 1}}}]}}]
> > +])
> > +
>
> I really would like to see a test with only the mandatory TVLs present, one 
> with multiple management IPs, and a test with the output from multiple ports. 
> This to increase the code coverage (preferably for all the TLV variants a 
> test).
Yes, more tests will be added.

>
> > +AT_CLEANUP
> > \ No newline at end of file
> > diff --git a/tests/testsuite.at b/tests/testsuite.at
> > index 9d77a9f51..f80656076 100644
> > --- a/tests/testsuite.at
> > +++ b/tests/testsuite.at
> > @@ -66,6 +66,7 @@ m4_include([tests/ofproto-dpif.at])
> >  m4_include([tests/bridge.at])
> >  m4_include([tests/netdev-type.at])
> >  m4_include([tests/ovsdb.at])
> > +m4_include([tests/ovs-lldp.at])
> >  m4_include([tests/ovs-vsctl.at])
> >  m4_include([tests/stp.at])
> >  m4_include([tests/rstp.at])
> > --
> > 2.43.5
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to