Skip to content

Configure LLDP advertising on physical NICs#7131

Open
minglumlu wants to merge 5 commits into
xapi-project:feature/lldpfrom
minglumlu:private/mingl/CP-312088
Open

Configure LLDP advertising on physical NICs#7131
minglumlu wants to merge 5 commits into
xapi-project:feature/lldpfrom
minglumlu:private/mingl/CP-312088

Conversation

@minglumlu

Copy link
Copy Markdown
Member

This is the first PR of LLDP feature. The overall design doc is https://github.com/xapi-project/xen-api/blob/master/doc/content/design/lldp.md

The PR is for managing a LLDP agent (and one implementation lldpd) and configuring for individual NICs through LLDP client. Cache is used to avoid unnecessary calls of LLDP client as most of configurations are shared by NICs.

Signed-off-by: Ming Lu <ming.lu@cloud.com>
@minglumlu minglumlu requested a review from psafont June 17, 2026 02:22
Add an Lldp module that manages lldpd via lldpcli to advertise the
LLDP information for individual physical NICs on a host. This is done by
the networkd interface `Interface.make_config`.

lldpd is a user space LLDP agent running in dom0.
lldpcli is a client to configure/query LLDP TLVs to/from lldpd.

Signed-off-by: Ming Lu <ming.lu@cloud.com>
One of the advertising LLDP TLV is management address(es) of the host.
This data is shared by the NICs with LLDP feature enabled.
However, the address(es) on management interface on a host may change.
This change can be caught by network_monitor_thread. Consequently, a
callback is carried out to update this specific LLDP TLV configuration.

Signed-off-by: Ming Lu <ming.lu@cloud.com>
This is for the safety of using CNA or hardware iSCSI in which case the
NIC and a storage function share the same hardware. Enabling LLDP on
such NICs may fail the corresponding storage function.

Users can enable it with PIF level configuration if it is confirmed no
impact on the storage function.

Signed-off-by: Ming Lu <ming.lu@cloud.com>
Some NICs run an LLDP agent in firmware, which would conflict with
lldpd. Before enabling LLDP on an interface, disable the firmware agent
on a best-effort basis via ethtool private flags ("disable-fw-lldp" and
"fw-lldp-agent").

The "best-effort" means that not all NICs support firmware LLDP; for the
supported ones, not all can be turned off from dom0. Users can turn off
the firmware LLDP from UEFI host settings.

Overall, the conflict is not supposed to impact other functions. So it's
a try on best-effort.

Signed-off-by: Ming Lu <ming.lu@cloud.com>
@minglumlu minglumlu force-pushed the private/mingl/CP-312088 branch from c855628 to 211e0d6 Compare June 17, 2026 02:41
Comment on lines +39 to +46
type conf =
| Chassis_id of chassis_id
| Port_id of dev * port_id
| Port_description of dev * port_description
| System_name of string
| System_description of string
| System_capability of system_capability list
| Multicast_address of I.lldp_multicast_address list

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't particularly see the point of conf being a list of these variants - why not a record? As I understand, each of these variants can only be applied once, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of them, management_address, may be set separately due to network change. And once it happens, other confs should not be changed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is already ensured by the cache, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache is unrelated here. The usual path is all of these confs is set for individual NICs. But another path is that the management_address alone could be set by the monitor thread in which case constructing all fields is awkward.

let cache_mc_addr : conf option Atomic.t = Atomic.make None

let cache_mgmt_addr: conf option Atomic.t = Atomic.make None
let cache_mgmt_addr : conf option Atomic.t = Atomic.make None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting changes that belong to the previous commit here and above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants