Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
> >>> +if (id != NULL) {
> >>
> >>It's safe to call free(NULL), so , please, don't check.
> > [Rick] Does it mean that we override the original free() method, so that it
> > won't crash when we call free(NULL)? If so, that is good and I don't need
> > to check here.
>
> It's part of a C standard starting at least from C89:
> """
> 4.10.3.2 The free function
> ...
> If ptr is a null pointer, no action occurs.
> """
>
> 'man 3 free' suggests the same.
It's also part of the coding style document (more for Rick than for
Ilya):
Functions that destroy an instance of a dynamically-allocated type should
accept and ignore a null pointer argument. Code that calls such a function
(including the C standard library function ``free()``) should omit a
null-pointer check. We find that this usually makes code easier to read.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
"Rick Zhong" writes:
> Hi Ilya,
>
> I took your advice and will submit a new merge request.
Please send an email with a link to the pull request. It's best if you
can use 'git send-email' to send the series to the list instead.
> As to the existing "autoattach" commands, it requires DCBX enabled on peer
> device. Otherwise, nothing is shown by the
> command.
> That's why I didn't merge into it.
>
> Best regards,
> Rick Zhong
>
> At 2021-04-08 01:55:20, "Ilya Maximets" wrote:
>>On 4/7/21 7:08 PM, Rick Zhong wrote:
>>> Hi Ilya,
>>>
>>> Many thanks for your attention on this and reply. Please see my comments
>>> inline.
>>>
>>>
>>> And I will try the 'git' commands as you mentioned.
>>>
>>> Best regards,
>>>
>>> Rick Zhong
>>>
>>>
>>> At 2021-04-07 20:21:19, "Ilya Maximets" wrote:
On 3/24/21 4:56 AM, Rick Zhong wrote:
> Dear OVS reviewers/supervisors:
>
>
> This patch is related to LLDP which provides a new command "ovs-appctl
> lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
> interfaces.
>
>
> With this new command, user is enable to get LLDP neighbor info even if
> not in SPB network.
>
>
> One limitation is that when multiple peer Management IP addresses are
> found by LLDP, only one Management IP address is displayed by the command.
>
>
> The patch is well-tested on Linux.
>
>
> Related commit: af4b3d3 and e4bc70c (add new command to show LLDP
> neighbor info #349)
>
>
> Signed-off-by: Rick Zhong (winsome8...@163.com)
Hi. Thanks for working on this!
The patch format is a bit unusual, you might want to consider using
'git format-patch' and 'git send-email' to send patches to the mail-list.
Aaron, could you take a look at this change from the LLDP perspective?
Few comments inline.
>
>
> =
> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
> index 05c1dd434..4c8ab9126 100644
> --- a/lib/ovs-lldp.c
> +++ b/lib/ovs-lldp.c
> @@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp
> *lldp) OVS_REQUIRES(mutex)
> }
> }
>
> +static void
> +lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
> +{
> +struct lldpd_port *port;
> +
> +LIST_FOR_EACH (port, p_entries, >h_rports) {
> +const char *none_str = "";
Can we use "" here like other commands does?
>>> [Rick] Sure. No problem.
>>>
> +char *id = NULL;
> +const char *name = NULL;
> +const char *port_id = NULL;
> +char ipaddress[INET6_ADDRSTRLEN + 1];
> +memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
> +
> +if (port->p_chassis) {
> +if (port->p_chassis->c_id_len > 0) {
> +chassisid_to_string(port->p_chassis->c_id,
> +port->p_chassis->c_id_len, );
> +}
> +
> +name = port->p_chassis->c_name;
> +
> +struct lldpd_mgmt *mgmt;
> +LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
> +int af;
> +size_t alen;
> +switch (mgmt->m_family) {
> +case LLDPD_AF_IPV4:
> +alen = INET_ADDRSTRLEN + 1;
> +af = AF_INET;
> +break;
> +case LLDPD_AF_IPV6:
> +alen = INET6_ADDRSTRLEN + 1;
> +af = AF_INET6;
> +break;
> +default:
> +continue;
> +}
> +
> +if (inet_ntop(af, >m_addr, ipaddress, alen) ==
> NULL) {
> +continue;
> +}
> +break;
OVS already has some formatting functions that converts ip addresses
to strings, so it's better to use them. For this particular case,
I think, we can use some thing like this:
struct in6_addr ip = in6addr_any;
...
LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
switch (mgmt->m_family) {
case LLDPD_AF_IPV4:
in6_addr_set_mapped_ipv4(, >m_addr.inet);
break;
case LLDPD_AF_IPV6:
ip = mgmt->m_addr.inet6;
break;
default:
continue;
}
}
...
ds_put_cstr(ds, " Neighbor Management IP: ");
ipv6_format_mapped(, ds);
ds_put_char(ds, "\n");
>>> [Rick] Thanks for your example. Actually this part of IP conversion codes
>>> were copied from another function somewhere:-) I will try
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
Hi Ilya,
I took your advice and will submit a new merge request.
As to the existing "autoattach" commands, it requires DCBX enabled on peer
device. Otherwise, nothing is shown by the command.
That's why I didn't merge into it.
Best regards,
Rick Zhong
At 2021-04-08 01:55:20, "Ilya Maximets" wrote:
>On 4/7/21 7:08 PM, Rick Zhong wrote:
>> Hi Ilya,
>>
>> Many thanks for your attention on this and reply. Please see my comments
>> inline.
>>
>>
>> And I will try the 'git' commands as you mentioned.
>>
>> Best regards,
>>
>> Rick Zhong
>>
>>
>> At 2021-04-07 20:21:19, "Ilya Maximets" wrote:
>>>On 3/24/21 4:56 AM, Rick Zhong wrote:
Dear OVS reviewers/supervisors:
This patch is related to LLDP which provides a new command "ovs-appctl
lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
interfaces.
With this new command, user is enable to get LLDP neighbor info even if
not in SPB network.
One limitation is that when multiple peer Management IP addresses are
found by LLDP, only one Management IP address is displayed by the command.
The patch is well-tested on Linux.
Related commit: af4b3d3 and e4bc70c (add new command to show LLDP neighbor
info #349)
Signed-off-by: Rick Zhong (winsome8...@163.com)
>>>
>>>Hi. Thanks for working on this!
>>>The patch format is a bit unusual, you might want to consider using
>>>'git format-patch' and 'git send-email' to send patches to the mail-list.
>>>
>>>Aaron, could you take a look at this change from the LLDP perspective?
>>>
>>>Few comments inline.
>>>
=
diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index 05c1dd434..4c8ab9126 100644
--- a/lib/ovs-lldp.c
+++ b/lib/ovs-lldp.c
@@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp
*lldp) OVS_REQUIRES(mutex)
}
}
+static void
+lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
+{
+struct lldpd_port *port;
+
+LIST_FOR_EACH (port, p_entries, >h_rports) {
+const char *none_str = "";
>>>
>>>Can we use "" here like other commands does?
>> [Rick] Sure. No problem.
>>
+char *id = NULL;
+const char *name = NULL;
+const char *port_id = NULL;
+char ipaddress[INET6_ADDRSTRLEN + 1];
+memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
+
+if (port->p_chassis) {
+if (port->p_chassis->c_id_len > 0) {
+chassisid_to_string(port->p_chassis->c_id,
+port->p_chassis->c_id_len, );
+}
+
+name = port->p_chassis->c_name;
+
+struct lldpd_mgmt *mgmt;
+LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
+int af;
+size_t alen;
+switch (mgmt->m_family) {
+case LLDPD_AF_IPV4:
+alen = INET_ADDRSTRLEN + 1;
+af = AF_INET;
+break;
+case LLDPD_AF_IPV6:
+alen = INET6_ADDRSTRLEN + 1;
+af = AF_INET6;
+break;
+default:
+continue;
+}
+
+if (inet_ntop(af, >m_addr, ipaddress, alen) ==
NULL) {
+continue;
+}
+break;
>>>
>>>OVS already has some formatting functions that converts ip addresses
>>>to strings, so it's better to use them. For this particular case,
>>>I think, we can use some thing like this:
>>>
>>>struct in6_addr ip = in6addr_any;
>>>...
>>>
>>>LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
>>>switch (mgmt->m_family) {
>>>case LLDPD_AF_IPV4:
>>>in6_addr_set_mapped_ipv4(, >m_addr.inet);
>>>break;
>>>case LLDPD_AF_IPV6:
>>>ip = mgmt->m_addr.inet6;
>>>break;
>>>default:
>>>continue;
>>>}
>>>}
>>>
>>>...
>>>ds_put_cstr(ds, " Neighbor Management IP: ");
>>>ipv6_format_mapped(, ds);
>>>ds_put_char(ds, "\n");
>> [Rick] Thanks for your example. Actually this part of IP conversion codes
>> were copied from another function somewhere:-) I will try your codes and
>> make a test.
>>
+}
+}
+
+port_id = port->p_id;
>>
>>
>>>This copy seems unnecessary.
>> [Rick] Ok. I will remove it.
>>
+
+ds_put_format(ds, " Neighbor Chassis ID: %s\n",
+ id ? id : none_str);
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
On 4/7/21 7:08 PM, Rick Zhong wrote:
> Hi Ilya,
>
> Many thanks for your attention on this and reply. Please see my comments
> inline.
>
>
> And I will try the 'git' commands as you mentioned.
>
> Best regards,
>
> Rick Zhong
>
>
> At 2021-04-07 20:21:19, "Ilya Maximets" wrote:
>>On 3/24/21 4:56 AM, Rick Zhong wrote:
>>> Dear OVS reviewers/supervisors:
>>>
>>>
>>> This patch is related to LLDP which provides a new command "ovs-appctl
>>> lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
>>> interfaces.
>>>
>>>
>>> With this new command, user is enable to get LLDP neighbor info even if not
>>> in SPB network.
>>>
>>>
>>> One limitation is that when multiple peer Management IP addresses are found
>>> by LLDP, only one Management IP address is displayed by the command.
>>>
>>>
>>> The patch is well-tested on Linux.
>>>
>>>
>>> Related commit: af4b3d3 and e4bc70c (add new command to show LLDP neighbor
>>> info #349)
>>>
>>>
>>> Signed-off-by: Rick Zhong (winsome8...@163.com)
>>
>>Hi. Thanks for working on this!
>>The patch format is a bit unusual, you might want to consider using
>>'git format-patch' and 'git send-email' to send patches to the mail-list.
>>
>>Aaron, could you take a look at this change from the LLDP perspective?
>>
>>Few comments inline.
>>
>>>
>>>
>>> =
>>> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
>>> index 05c1dd434..4c8ab9126 100644
>>> --- a/lib/ovs-lldp.c
>>> +++ b/lib/ovs-lldp.c
>>> @@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
>>> OVS_REQUIRES(mutex)
>>> }
>>> }
>>>
>>> +static void
>>> +lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
>>> +{
>>> +struct lldpd_port *port;
>>> +
>>> +LIST_FOR_EACH (port, p_entries, >h_rports) {
>>> +const char *none_str = "";
>>
>>Can we use "" here like other commands does?
> [Rick] Sure. No problem.
>
>>> +char *id = NULL;
>>> +const char *name = NULL;
>>> +const char *port_id = NULL;
>>> +char ipaddress[INET6_ADDRSTRLEN + 1];
>>> +memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
>>> +
>>> +if (port->p_chassis) {
>>> +if (port->p_chassis->c_id_len > 0) {
>>> +chassisid_to_string(port->p_chassis->c_id,
>>> +port->p_chassis->c_id_len, );
>>> +}
>>> +
>>> +name = port->p_chassis->c_name;
>>> +
>>> +struct lldpd_mgmt *mgmt;
>>> +LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
>>> +int af;
>>> +size_t alen;
>>> +switch (mgmt->m_family) {
>>> +case LLDPD_AF_IPV4:
>>> +alen = INET_ADDRSTRLEN + 1;
>>> +af = AF_INET;
>>> +break;
>>> +case LLDPD_AF_IPV6:
>>> +alen = INET6_ADDRSTRLEN + 1;
>>> +af = AF_INET6;
>>> +break;
>>> +default:
>>> +continue;
>>> +}
>>> +
>>> +if (inet_ntop(af, >m_addr, ipaddress, alen) == NULL)
>>> {
>>> +continue;
>>> +}
>>> +break;
>>
>>OVS already has some formatting functions that converts ip addresses
>>to strings, so it's better to use them. For this particular case,
>>I think, we can use some thing like this:
>>
>>struct in6_addr ip = in6addr_any;
>>...
>>
>>LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
>>switch (mgmt->m_family) {
>>case LLDPD_AF_IPV4:
>>in6_addr_set_mapped_ipv4(, >m_addr.inet);
>>break;
>>case LLDPD_AF_IPV6:
>>ip = mgmt->m_addr.inet6;
>>break;
>>default:
>>continue;
>>}
>>}
>>
>>...
>>ds_put_cstr(ds, " Neighbor Management IP: ");
>>ipv6_format_mapped(, ds);
>>ds_put_char(ds, "\n");
> [Rick] Thanks for your example. Actually this part of IP conversion codes
> were copied from another function somewhere:-) I will try your codes and make
> a test.
>
>>> +}
>>> +}
>>> +
>>> +port_id = port->p_id;
>
>
>>This copy seems unnecessary.
> [Rick] Ok. I will remove it.
>
>>> +
>>> +ds_put_format(ds, " Neighbor Chassis ID: %s\n",
>>> + id ? id : none_str);
>>> +ds_put_format(ds, " Neighbor Chassis SysName: %s\n",
>>> + name ? name : none_str);
>>> +ds_put_format(ds, " Neighbor Management IP: %s\n",
>>> + ipaddress);
>>> +ds_put_format(ds, " Neighbor Port ID: %s\n",
>>> + port_id ? port_id : none_str);
>>> +
>>> +if (id != NULL) {
>>
>>It's safe to call free(NULL), so , please, don't check.
> [Rick] Does it mean
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
Hi Ilya,
Many thanks for your attention on this and reply. Please see my comments inline.
And I will try the 'git' commands as you mentioned.
Best regards,
Rick Zhong
At 2021-04-07 20:21:19, "Ilya Maximets" wrote:
>On 3/24/21 4:56 AM, Rick Zhong wrote:
>> Dear OVS reviewers/supervisors:
>>
>>
>> This patch is related to LLDP which provides a new command "ovs-appctl
>> lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
>> interfaces.
>>
>>
>> With this new command, user is enable to get LLDP neighbor info even if not
>> in SPB network.
>>
>>
>> One limitation is that when multiple peer Management IP addresses are found
>> by LLDP, only one Management IP address is displayed by the command.
>>
>>
>> The patch is well-tested on Linux.
>>
>>
>> Related commit: af4b3d3 and e4bc70c (add new command to show LLDP neighbor
>> info #349)
>>
>>
>> Signed-off-by: Rick Zhong (winsome8...@163.com)
>
>Hi. Thanks for working on this!
>The patch format is a bit unusual, you might want to consider using
>'git format-patch' and 'git send-email' to send patches to the mail-list.
>
>Aaron, could you take a look at this change from the LLDP perspective?
>
>Few comments inline.
>
>>
>>
>> =
>> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
>> index 05c1dd434..4c8ab9126 100644
>> --- a/lib/ovs-lldp.c
>> +++ b/lib/ovs-lldp.c
>> @@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
>> OVS_REQUIRES(mutex)
>> }
>> }
>>
>> +static void
>> +lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
>> +{
>> +struct lldpd_port *port;
>> +
>> +LIST_FOR_EACH (port, p_entries, >h_rports) {
>> +const char *none_str = "";
>
>Can we use "" here like other commands does?
[Rick] Sure. No problem.
>> +char *id = NULL;
>> +const char *name = NULL;
>> +const char *port_id = NULL;
>> +char ipaddress[INET6_ADDRSTRLEN + 1];
>> +memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
>> +
>> +if (port->p_chassis) {
>> +if (port->p_chassis->c_id_len > 0) {
>> +chassisid_to_string(port->p_chassis->c_id,
>> +port->p_chassis->c_id_len, );
>> +}
>> +
>> +name = port->p_chassis->c_name;
>> +
>> +struct lldpd_mgmt *mgmt;
>> +LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
>> +int af;
>> +size_t alen;
>> +switch (mgmt->m_family) {
>> +case LLDPD_AF_IPV4:
>> +alen = INET_ADDRSTRLEN + 1;
>> +af = AF_INET;
>> +break;
>> +case LLDPD_AF_IPV6:
>> +alen = INET6_ADDRSTRLEN + 1;
>> +af = AF_INET6;
>> +break;
>> +default:
>> +continue;
>> +}
>> +
>> +if (inet_ntop(af, >m_addr, ipaddress, alen) == NULL) {
>> +continue;
>> +}
>> +break;
>
>OVS already has some formatting functions that converts ip addresses
>to strings, so it's better to use them. For this particular case,
>I think, we can use some thing like this:
>
>struct in6_addr ip = in6addr_any;
>...
>
>LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
>switch (mgmt->m_family) {
>case LLDPD_AF_IPV4:
>in6_addr_set_mapped_ipv4(, >m_addr.inet);
>break;
>case LLDPD_AF_IPV6:
>ip = mgmt->m_addr.inet6;
>break;
>default:
>continue;
>}
>}
>
>...
>ds_put_cstr(ds, " Neighbor Management IP: ");
>ipv6_format_mapped(, ds);
>ds_put_char(ds, "\n");
[Rick] Thanks for your example. Actually this part of IP conversion codes were
copied from another function somewhere:-) I will try your codes and make a test.
>> +}
>> +}
>> +
>> +port_id = port->p_id;
>This copy seems unnecessary.
[Rick] Ok. I will remove it.
>> +
>> +ds_put_format(ds, " Neighbor Chassis ID: %s\n",
>> + id ? id : none_str);
>> +ds_put_format(ds, " Neighbor Chassis SysName: %s\n",
>> + name ? name : none_str);
>> +ds_put_format(ds, " Neighbor Management IP: %s\n",
>> + ipaddress);
>> +ds_put_format(ds, " Neighbor Port ID: %s\n",
>> + port_id ? port_id : none_str);
>> +
>> +if (id != NULL) {
>
>It's safe to call free(NULL), so , please, don't check.
[Rick] Does it mean that we override the original free() method, so that it
won't crash when we call free(NULL)? If so, that is good and I don't need to
check here.
>> +free(id);
>> +}
>> +}
>> +}
>> +
>>
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
Hi Ilya and Ben, Actually I was thinking how to make documentation for such kind of user commands change. Sure, I will provide it as you mentioned. Best regards, Rick Zhong At 2021-04-07 23:11:00, "Ilya Maximets" wrote: >On 4/7/21 4:40 PM, Ben Pfaff wrote: >> On Wed, Apr 07, 2021 at 02:21:19PM +0200, Ilya Maximets wrote: >>> Also, it seems like none of these commands documented anywhere. >>> We will need to fix that someday. >> >> I'd recommend requiring documentation for anything being added, so the >> situation doesn't get worse. > >Good point. > >> >> We usually document this kind of command in ovs-vswitchd(8), when we >> remember. >> > >Rick, could you, please, add a short description of your new command >to 'RUNTIME MANAGEMENT COMMANDS' section of vswitchd/ovs-vswitchd.8.in? > >Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
On 4/7/21 4:40 PM, Ben Pfaff wrote: > On Wed, Apr 07, 2021 at 02:21:19PM +0200, Ilya Maximets wrote: >> Also, it seems like none of these commands documented anywhere. >> We will need to fix that someday. > > I'd recommend requiring documentation for anything being added, so the > situation doesn't get worse. Good point. > > We usually document this kind of command in ovs-vswitchd(8), when we > remember. > Rick, could you, please, add a short description of your new command to 'RUNTIME MANAGEMENT COMMANDS' section of vswitchd/ovs-vswitchd.8.in? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
On Wed, Apr 07, 2021 at 02:21:19PM +0200, Ilya Maximets wrote: > Also, it seems like none of these commands documented anywhere. > We will need to fix that someday. I'd recommend requiring documentation for anything being added, so the situation doesn't get worse. We usually document this kind of command in ovs-vswitchd(8), when we remember. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
On 3/24/21 4:56 AM, Rick Zhong wrote:
> Dear OVS reviewers/supervisors:
>
>
> This patch is related to LLDP which provides a new command "ovs-appctl
> lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
> interfaces.
>
>
> With this new command, user is enable to get LLDP neighbor info even if not
> in SPB network.
>
>
> One limitation is that when multiple peer Management IP addresses are found
> by LLDP, only one Management IP address is displayed by the command.
>
>
> The patch is well-tested on Linux.
>
>
> Related commit: af4b3d3 and e4bc70c (add new command to show LLDP neighbor
> info #349)
>
>
> Signed-off-by: Rick Zhong (winsome8...@163.com)
Hi. Thanks for working on this!
The patch format is a bit unusual, you might want to consider using
'git format-patch' and 'git send-email' to send patches to the mail-list.
Aaron, could you take a look at this change from the LLDP perspective?
Few comments inline.
>
>
> =
> diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
> index 05c1dd434..4c8ab9126 100644
> --- a/lib/ovs-lldp.c
> +++ b/lib/ovs-lldp.c
> @@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
> OVS_REQUIRES(mutex)
> }
> }
>
> +static void
> +lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
> +{
> +struct lldpd_port *port;
> +
> +LIST_FOR_EACH (port, p_entries, >h_rports) {
> +const char *none_str = "";
Can we use "" here like other commands does?
> +char *id = NULL;
> +const char *name = NULL;
> +const char *port_id = NULL;
> +char ipaddress[INET6_ADDRSTRLEN + 1];
> +memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
> +
> +if (port->p_chassis) {
> +if (port->p_chassis->c_id_len > 0) {
> +chassisid_to_string(port->p_chassis->c_id,
> +port->p_chassis->c_id_len, );
> +}
> +
> +name = port->p_chassis->c_name;
> +
> +struct lldpd_mgmt *mgmt;
> +LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
> +int af;
> +size_t alen;
> +switch (mgmt->m_family) {
> +case LLDPD_AF_IPV4:
> +alen = INET_ADDRSTRLEN + 1;
> +af = AF_INET;
> +break;
> +case LLDPD_AF_IPV6:
> +alen = INET6_ADDRSTRLEN + 1;
> +af = AF_INET6;
> +break;
> +default:
> +continue;
> +}
> +
> +if (inet_ntop(af, >m_addr, ipaddress, alen) == NULL) {
> +continue;
> +}
> +break;
OVS already has some formatting functions that converts ip addresses
to strings, so it's better to use them. For this particular case,
I think, we can use some thing like this:
struct in6_addr ip = in6addr_any;
...
LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
switch (mgmt->m_family) {
case LLDPD_AF_IPV4:
in6_addr_set_mapped_ipv4(, >m_addr.inet);
break;
case LLDPD_AF_IPV6:
ip = mgmt->m_addr.inet6;
break;
default:
continue;
}
}
...
ds_put_cstr(ds, " Neighbor Management IP: ");
ipv6_format_mapped(, ds);
ds_put_char(ds, "\n");
> +}
> +}
> +
> +port_id = port->p_id;
This copy seems unnecessary.
> +
> +ds_put_format(ds, " Neighbor Chassis ID: %s\n",
> + id ? id : none_str);
> +ds_put_format(ds, " Neighbor Chassis SysName: %s\n",
> + name ? name : none_str);
> +ds_put_format(ds, " Neighbor Management IP: %s\n",
> + ipaddress);
> +ds_put_format(ds, " Neighbor Port ID: %s\n",
> + port_id ? port_id : none_str);
> +
> +if (id != NULL) {
It's safe to call free(NULL), so , please, don't check.
> +free(id);
> +}
> +}
> +}
> +
> +static void
> +lldp_print_neighbor(struct ds *ds, struct lldp *lldp) OVS_REQUIRES(mutex)
> +{
> +struct lldpd_hardware *hw;
> +
> +ds_put_format(ds, "LLDP: %s\n", lldp->name);
> +
> +if (!lldp->lldpd) {
> +return;
> +}
> +
> +LIST_FOR_EACH (hw, h_entries, >lldpd->g_hardware) {
> +lldp_print_neighbor_port(ds, hw);
> +}
> +}
> +
> static void
> aa_unixctl_status(struct unixctl_conn *conn, int argc OVS_UNUSED,
>const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> @@ -382,6 +460,25 @@ aa_unixctl_statistics(struct unixctl_conn *conn, int
> argc OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr());
> }
>
> +static void
> +lldp_unixctl_show_neighbor(struct unixctl_conn *conn, int argc
[ovs-dev] [PATCH] LLDP: add new command to show LLDP neighbor info
Dear OVS reviewers/supervisors:
This patch is related to LLDP which provides a new command "ovs-appctl
lldp/neighbor" to show LLDP neighbor info when LLDP is enabled on OVS
interfaces.
With this new command, user is enable to get LLDP neighbor info even if not in
SPB network.
One limitation is that when multiple peer Management IP addresses are found by
LLDP, only one Management IP address is displayed by the command.
The patch is well-tested on Linux.
Related commit: af4b3d3 and e4bc70c (add new command to show LLDP neighbor info
#349)
Signed-off-by: Rick Zhong (winsome8...@163.com)
=
diff --git a/lib/ovs-lldp.c b/lib/ovs-lldp.c
index 05c1dd434..4c8ab9126 100644
--- a/lib/ovs-lldp.c
+++ b/lib/ovs-lldp.c
@@ -324,6 +324,84 @@ aa_print_isid_status(struct ds *ds, struct lldp *lldp)
OVS_REQUIRES(mutex)
}
}
+static void
+lldp_print_neighbor_port(struct ds *ds, struct lldpd_hardware *hw)
+{
+struct lldpd_port *port;
+
+LIST_FOR_EACH (port, p_entries, >h_rports) {
+const char *none_str = "";
+char *id = NULL;
+const char *name = NULL;
+const char *port_id = NULL;
+char ipaddress[INET6_ADDRSTRLEN + 1];
+memset(ipaddress, 0, INET6_ADDRSTRLEN + 1);
+
+if (port->p_chassis) {
+if (port->p_chassis->c_id_len > 0) {
+chassisid_to_string(port->p_chassis->c_id,
+port->p_chassis->c_id_len, );
+}
+
+name = port->p_chassis->c_name;
+
+struct lldpd_mgmt *mgmt;
+LIST_FOR_EACH (mgmt, m_entries, >p_chassis->c_mgmt) {
+int af;
+size_t alen;
+switch (mgmt->m_family) {
+case LLDPD_AF_IPV4:
+alen = INET_ADDRSTRLEN + 1;
+af = AF_INET;
+break;
+case LLDPD_AF_IPV6:
+alen = INET6_ADDRSTRLEN + 1;
+af = AF_INET6;
+break;
+default:
+continue;
+}
+
+if (inet_ntop(af, >m_addr, ipaddress, alen) == NULL) {
+continue;
+}
+break;
+}
+}
+
+port_id = port->p_id;
+
+ds_put_format(ds, " Neighbor Chassis ID: %s\n",
+ id ? id : none_str);
+ds_put_format(ds, " Neighbor Chassis SysName: %s\n",
+ name ? name : none_str);
+ds_put_format(ds, " Neighbor Management IP: %s\n",
+ ipaddress);
+ds_put_format(ds, " Neighbor Port ID: %s\n",
+ port_id ? port_id : none_str);
+
+if (id != NULL) {
+free(id);
+}
+}
+}
+
+static void
+lldp_print_neighbor(struct ds *ds, struct lldp *lldp) OVS_REQUIRES(mutex)
+{
+struct lldpd_hardware *hw;
+
+ds_put_format(ds, "LLDP: %s\n", lldp->name);
+
+if (!lldp->lldpd) {
+return;
+}
+
+LIST_FOR_EACH (hw, h_entries, >lldpd->g_hardware) {
+lldp_print_neighbor_port(ds, hw);
+}
+}
+
static void
aa_unixctl_status(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
@@ -382,6 +460,25 @@ aa_unixctl_statistics(struct unixctl_conn *conn, int argc
OVS_UNUSED,
unixctl_command_reply(conn, ds_cstr());
}
+static void
+lldp_unixctl_show_neighbor(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
+OVS_EXCLUDED(mutex)
+{
+struct lldp *lldp;
+struct ds ds = DS_EMPTY_INITIALIZER;
+
+ovs_mutex_lock();
+
+HMAP_FOR_EACH (lldp, hmap_node, all_lldps) {
+lldp_print_neighbor(, lldp);
+}
+unixctl_command_reply(conn, ds_cstr());
+ds_destroy();
+
+ovs_mutex_unlock();
+}
+
/* An Auto Attach mapping was configured. Populate the corresponding
* structures in the LLDP hardware.
*/
@@ -649,6 +746,8 @@ lldp_init(void)
aa_unixctl_show_isid, NULL);
unixctl_command_register("autoattach/statistics", "[bridge]", 0, 1,
aa_unixctl_statistics, NULL);
+unixctl_command_register("lldp/neighbor", "[bridge]", 0, 1,
+ lldp_unixctl_show_neighbor, NULL);
}
/* Returns true if 'lldp' should process packets from 'flow'. Sets
=
Regards,
Rick Zhong
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
