PD Interface Adjust / LLPD name Of station mode #319

olbjo · 2021-01-28T15:25:56Z

Add PD Interface Adjust

Handle lldp name of station mode
Default name of station mode set to standard
In lldp replace zero-len station name with mac
Update tests

pyhys

Great!
I think we should modify src/common/pf_snmp.c:


void pf_snmp_get_system_description (
   pnet_t * net,
   pf_snmp_system_description_t * description)
{
   (void)pf_lldp_get_system_description (net, description->string, sizeof (description->string));
}

src/common/pf_lldp.c

src/common/pf_lldp.h

src/common/pf_lldp.c

src/device/pf_pdport.c

pyhys

Great!
Just a few minor comments (I tried to make it copy-paste friendly)

src/common/pf_lldp.c

src/common/pf_lldp.h

src/common/pf_lldp.c

src/common/pf_lldp.h

src/device/pf_port.h

olbjo · 2021-02-01T12:37:54Z

Some of the suggested updates did not compile. I guess I never saved before local build and test.
Functions using internally. pf_port_get_state() can not have a const net pointer as argument. This applies for some other functions in lldp as well

olbjo · 2021-02-02T12:28:38Z

Rebased on current to resolve conflicts

fredrikdanielmoller

Reviewed changes for LLDP, pf_port and header files, but not pf_pdport and pf_block_*.

src/pf_types.h

fredrikdanielmoller · 2021-02-01T13:14:54Z

src/pf_types.h

+
+/**
+ * LLDP Port Name
+ * In standard name of device mode this is first part of Port ID


Suggested change

* In standard name of device mode this is first part of Port ID

* In standard mode this is first part of Port ID.

name-of-device mode

I will go with name-of-device mode so it is made clear which mode we are referring to

src/pf_types.h

src/common/pf_lldp.h

src/common/pf_lldp.c

fredrikdanielmoller · 2021-02-02T15:21:51Z

src/common/pf_lldp.c

+
+   if (port_id.is_valid)
+   {
+      for (i = 0; i < port_id.len && port_id.string[i] != '\0'; i++)


Perhaps use strchr() instead?

I tried but the implementation got a bit messy and I will stick with current version.
I added an additional check of the i-index so that we don't write outside portname if port_id is not correctly formatted. Please check that out.
Done

sample_app/sampleapp_common.c

src/device/pf_pdport.c

src/device/pf_block_reader.c

src/common/pf_lldp.h

src/common/pf_lldp.c

fredrikdanielmoller · 2021-02-03T14:43:53Z

src/common/pf_lldp.c

+   if (port_id.is_valid)
+   {
+      for (i = 0; (i < port_id.len) && (port_id.string[i] != '\0') &&
+                  (i < (sizeof (p_port_name->string) - 1));


Why was the condition i < (sizeof (p_port_name->string) - 1 introduced?
Given that i < port_id.len, this should always be true

port_id.string is 256 characters. I think I risk writing outside port_name->string if the port_id string is not formatted as it should be.

Sorry, I misread it as sizeof (port_id->string) instead of sizeof (p_port_name->string).

- Handle lldp name of station mode - Default name of station mode set to standard - In lldp replace zero-len station name with mac - Update tests

olbjo · 2021-02-04T15:12:54Z

Included in #318 which is now merged.

olbjo requested a review from pyhys January 28, 2021 15:26

pyhys reviewed Jan 28, 2021

View reviewed changes

olbjo force-pushed the pd_interface_adjust branch 3 times, most recently from b2768cd to 0989fe3 Compare January 29, 2021 13:52

olbjo marked this pull request as ready for review January 29, 2021 13:52

pyhys reviewed Feb 1, 2021

View reviewed changes

olbjo force-pushed the pd_interface_adjust branch from 0989fe3 to 156dbdc Compare February 1, 2021 10:11

pyhys approved these changes Feb 1, 2021

View reviewed changes

pyhys changed the title ~~WIP PD Interface Adjust / LLPD name Of station mode~~ PD Interface Adjust / LLPD name Of station mode Feb 1, 2021

olbjo force-pushed the pd_interface_adjust branch from 156dbdc to aabaaa2 Compare February 1, 2021 12:38

pyhys requested a review from fredrikdanielmoller February 1, 2021 12:46

olbjo force-pushed the pd_interface_adjust branch 3 times, most recently from 85c3fdf to 2874382 Compare February 2, 2021 12:24

fredrikdanielmoller suggested changes Feb 2, 2021

View reviewed changes

olbjo force-pushed the pd_interface_adjust branch from 2874382 to 6ab4654 Compare February 3, 2021 12:59

pyhys reviewed Feb 3, 2021

View reviewed changes

sample_app/sampleapp_common.c Outdated Show resolved Hide resolved

fredrikdanielmoller suggested changes Feb 3, 2021

View reviewed changes

Add PD Interface Adjust

3c470d6

- Handle lldp name of station mode - Default name of station mode set to standard - In lldp replace zero-len station name with mac - Update tests

olbjo force-pushed the pd_interface_adjust branch from 6ab4654 to 3c470d6 Compare February 3, 2021 16:59

fredrikdanielmoller approved these changes Feb 3, 2021

View reviewed changes

pyhys requested a review from hefloryd February 4, 2021 07:42

olbjo mentioned this pull request Feb 4, 2021

Multi port updated #318

Merged

olbjo closed this Feb 4, 2021

olbjo deleted the pd_interface_adjust branch May 19, 2021 06:33

	* In standard name of device mode this is first part of Port ID
	* In standard mode this is first part of Port ID.

PD Interface Adjust / LLPD name Of station mode #319

PD Interface Adjust / LLPD name Of station mode #319

Uh oh!

Conversation

olbjo commented Jan 28, 2021 • edited by pyhys Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pyhys left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pyhys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olbjo commented Feb 1, 2021

Uh oh!

olbjo commented Feb 2, 2021

Uh oh!

fredrikdanielmoller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olbjo commented Feb 4, 2021

Uh oh!

Uh oh!

olbjo commented Jan 28, 2021 •

edited by pyhys

Loading

pyhys left a comment •

edited

Loading