Discussion:
[Linuxptp-devel] [PATCH 3/6] send UNKNOWN_ID error for unknown management TLVs
Geoff Salmon
2013-02-04 18:05:38 UTC
Permalink
Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
port.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/port.c b/port.c
index 0b58fbd..f879bbb 100644
--- a/port.c
+++ b/port.c
@@ -1721,6 +1721,7 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
port_management_send_error(p, ingress, msg, NOT_SUPPORTED);
break;
default:
+ port_management_send_error(p, ingress, msg, NO_SUCH_ID);
return -1;
}
return 0;
--
1.8.1.2
Geoff Salmon
2013-02-04 18:05:36 UTC
Permalink
Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 2 +-
port.c | 8 ++++----
port.h | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/clock.c b/clock.c
index 98e91d3..67271a9 100644
--- a/clock.c
+++ b/clock.c
@@ -670,7 +670,7 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case TRANSPARENT_CLOCK_DEFAULT_DATA_SET:
case PRIMARY_DOMAIN:
pid = port_identity(p);
- if (port_managment_error(pid, p, msg, NOT_SUPPORTED))
+ if (port_management_error(pid, p, msg, NOT_SUPPORTED))
pr_err("failed to send management error status");
break;
default:
diff --git a/port.c b/port.c
index 6c9bd32..6528cf9 100644
--- a/port.c
+++ b/port.c
@@ -1687,7 +1687,7 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
break;
case SET:
- if (port_managment_error(p->portIdentity, ingress, msg,
+ if (port_management_error(p->portIdentity, ingress, msg,
NOT_SUPPORTED))
pr_err("port %hu: management error failed", portnum(p));
break;
@@ -1713,7 +1713,7 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
case TRANSPARENT_CLOCK_PORT_DATA_SET:
case DELAY_MECHANISM:
case LOG_MIN_PDELAY_REQ_INTERVAL:
- if (port_managment_error(p->portIdentity, ingress, msg,
+ if (port_management_error(p->portIdentity, ingress, msg,
NOT_SUPPORTED))
pr_err("port %hu: management error failed", portnum(p));
break;
@@ -1723,8 +1723,8 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
}

-int port_managment_error(struct PortIdentity pid, struct port *ingress,
- struct ptp_message *req, Enumeration16 error_id)
+int port_management_error(struct PortIdentity pid, struct port *ingress,
+ struct ptp_message *req, Enumeration16 error_id)
{
struct ptp_message *msg;
struct management_tlv *mgt;
diff --git a/port.h b/port.h
index 74a08f7..ba4e612 100644
--- a/port.h
+++ b/port.h
@@ -116,8 +116,8 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg);
* @param error_id One of the management error ID values.
* @return Zero on success, non-zero otherwise.
*/
-int port_managment_error(struct PortIdentity pid, struct port *ingress,
- struct ptp_message *req, Enumeration16 error_id);
+int port_management_error(struct PortIdentity pid, struct port *ingress,
+ struct ptp_message *req, Enumeration16 error_id);

/**
* Allocate a reply to a management message.
--
1.8.1.2
Geoff Salmon
2013-02-04 18:05:37 UTC
Permalink
Adds port_management_send_error and clock_management_send_error to
avoid repeatedly checking the result of port_managment_send_error and
calling pr_err if it failed. Future patches send more mgmt errors so
this will avoid repeated code.

Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 12 ++++++++----
port.c | 15 +++++++++------
2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/clock.c b/clock.c
index 67271a9..14ea25e 100644
--- a/clock.c
+++ b/clock.c
@@ -131,6 +131,13 @@ static void clock_freq_est_reset(struct clock *c)
c->fest.count = 0;
};

+static void clock_management_send_error(struct port *p,
+ struct ptp_message *msg, int error_id)
+{
+ if (port_management_error(port_identity(p), p, msg, error_id))
+ pr_err("failed to send management error status");
+}
+
static int clock_management_get_response(struct clock *c, struct port *p,
int id, struct ptp_message *req)
{
@@ -610,7 +617,6 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
{
int i;
struct management_tlv *mgt;
- struct PortIdentity pid;
struct ClockIdentity *tcid, wildcard = {
{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
};
@@ -669,9 +675,7 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case ALTERNATE_TIME_OFFSET_PROPERTIES:
case TRANSPARENT_CLOCK_DEFAULT_DATA_SET:
case PRIMARY_DOMAIN:
- pid = port_identity(p);
- if (port_management_error(pid, p, msg, NOT_SUPPORTED))
- pr_err("failed to send management error status");
+ clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
default:
for (i = 0; i < c->nports; i++) {
diff --git a/port.c b/port.c
index 6528cf9..0b58fbd 100644
--- a/port.c
+++ b/port.c
@@ -431,6 +431,13 @@ static int port_is_ieee8021as(struct port *p)
return p->pod.follow_up_info ? 1 : 0;
}

+static void port_management_send_error(struct port *p, struct port *ingress,
+ struct ptp_message *msg, int error_id)
+{
+ if (port_management_error(p->portIdentity, ingress, msg, error_id))
+ pr_err("port %hu: management error failed", portnum(p));
+}
+
static int port_management_get_response(struct port *target,
struct port *ingress, int id,
struct ptp_message *req)
@@ -1687,9 +1694,7 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
break;
case SET:
- if (port_management_error(p->portIdentity, ingress, msg,
- NOT_SUPPORTED))
- pr_err("port %hu: management error failed", portnum(p));
+ port_management_send_error(p, ingress, msg, NOT_SUPPORTED);
break;
case COMMAND:
case RESPONSE:
@@ -1713,9 +1718,7 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
case TRANSPARENT_CLOCK_PORT_DATA_SET:
case DELAY_MECHANISM:
case LOG_MIN_PDELAY_REQ_INTERVAL:
- if (port_management_error(p->portIdentity, ingress, msg,
- NOT_SUPPORTED))
- pr_err("port %hu: management error failed", portnum(p));
+ port_management_send_error(p, ingress, msg, NOT_SUPPORTED);
break;
default:
return -1;
--
1.8.1.2
Geoff Salmon
2013-02-04 18:05:39 UTC
Permalink
Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 16 ++++++++++++++++
port.c | 17 ++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index 14ea25e..751a24d 100644
--- a/clock.c
+++ b/clock.c
@@ -211,6 +211,19 @@ out:
return respond ? 1 : 0;
}

+static int clock_management_set(struct clock *c, struct port *p,
+ int id, struct ptp_message *req)
+{
+ int respond = 0;
+ struct management_tlv *tlv = (struct management_tlv *) req->management.suffix;
+
+ switch (id) {
+ }
+ if (respond && !clock_management_get_response(c, p, id, req))
+ pr_err("failed to send management set response");
+ return respond ? 1 : 0;
+}
+
static int clock_master_lost(struct clock *c)
{
int i;
@@ -640,6 +653,9 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
return;
break;
case SET:
+ if (clock_management_set(c, p, mgt->id, msg))
+ return;
+ break;
case COMMAND:
break;
case RESPONSE:
diff --git a/port.c b/port.c
index f879bbb..1f6c803 100644
--- a/port.c
+++ b/port.c
@@ -497,6 +497,20 @@ out:
return respond ? 1 : 0;
}

+static int port_management_set(struct port *target,
+ struct port *ingress, int id,
+ struct ptp_message *req)
+{
+ int respond = 0;
+ struct management_tlv *tlv = (struct management_tlv *) req->management.suffix;
+
+ switch (id) {
+ }
+ if (respond && !port_management_get_response(target, ingress, id, req))
+ pr_err("port %hu: failed to send management set response", portnum(target));
+ return respond ? 1 : 0;
+}
+
static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
{
tmv_t origin2;
@@ -1694,7 +1708,8 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
break;
case SET:
- port_management_send_error(p, ingress, msg, NOT_SUPPORTED);
+ if (port_management_set(p, ingress, mgt->id, msg))
+ return 0;
break;
case COMMAND:
case RESPONSE:
--
1.8.1.2
Richard Cochran
2013-02-05 17:37:09 UTC
Permalink
Post by Geoff Salmon
---
clock.c | 16 ++++++++++++++++
port.c | 17 ++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
I get this warning (even after the entire series was applied)

clock.c: In function 'clock_management_set':
clock.c:218: warning: unused variable 'tlv'

and

port.c: In function 'port_management_set':
port.c:505: warning: unused variable 'tlv'

Thanks,
Richard
Geoff Salmon
2013-02-06 16:23:03 UTC
Permalink
Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 14 ++++++++++++++
port.c | 15 ++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index 14ea25e..3f0cf69 100644
--- a/clock.c
+++ b/clock.c
@@ -211,6 +211,17 @@ out:
return respond ? 1 : 0;
}

+static int clock_management_set(struct clock *c, struct port *p,
+ int id, struct ptp_message *req)
+{
+ int respond = 0;
+ switch (id) {
+ }
+ if (respond && !clock_management_get_response(c, p, id, req))
+ pr_err("failed to send management set response");
+ return respond ? 1 : 0;
+}
+
static int clock_master_lost(struct clock *c)
{
int i;
@@ -640,6 +651,9 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
return;
break;
case SET:
+ if (clock_management_set(c, p, mgt->id, msg))
+ return;
+ break;
case COMMAND:
break;
case RESPONSE:
diff --git a/port.c b/port.c
index 79651af..0403366 100644
--- a/port.c
+++ b/port.c
@@ -501,6 +501,18 @@ out:
return respond ? 1 : 0;
}

+static int port_management_set(struct port *target,
+ struct port *ingress, int id,
+ struct ptp_message *req)
+{
+ int respond = 0;
+ switch (id) {
+ }
+ if (respond && !port_management_get_response(target, ingress, id, req))
+ pr_err("port %hu: failed to send management set response", portnum(target));
+ return respond ? 1 : 0;
+}
+
static void port_nrate_calculate(struct port *p, tmv_t t3, tmv_t t4, tmv_t c)
{
tmv_t origin2;
@@ -1698,7 +1710,8 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
break;
case SET:
- port_management_send_error(p, ingress, msg, NOT_SUPPORTED);
+ if (port_management_set(p, ingress, mgt->id, msg))
+ return 0;
break;
case COMMAND:
case RESPONSE:
--
1.8.1.2
Geoff Salmon
2013-02-04 18:05:40 UTC
Permalink
Now that there are clock/port_management_set functions, the IDs that
GETs are handled for, like DEFUALT_DATA_SET, still need to be in the
case for sending NOT_SUPPORTED errors.

Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 9 ++++++---
port.c | 6 ++++--
2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/clock.c b/clock.c
index 751a24d..04a769b 100644
--- a/clock.c
+++ b/clock.c
@@ -658,9 +658,7 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
break;
case COMMAND:
break;
- case RESPONSE:
- case ACKNOWLEDGE:
- /* Ignore responses from other nodes. */
+ default:
return;
}

@@ -671,6 +669,10 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case INITIALIZE:
case FAULT_LOG:
case FAULT_LOG_RESET:
+ case DEFAULT_DATA_SET:
+ case CURRENT_DATA_SET:
+ case PARENT_DATA_SET:
+ case TIME_PROPERTIES_DATA_SET:
case PRIORITY1:
case PRIORITY2:
case DOMAIN:
@@ -691,6 +693,7 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
case ALTERNATE_TIME_OFFSET_PROPERTIES:
case TRANSPARENT_CLOCK_DEFAULT_DATA_SET:
case PRIMARY_DOMAIN:
+ case TIME_STATUS_NP:
clock_management_send_error(p, msg, NOT_SUPPORTED);
break;
default:
diff --git a/port.c b/port.c
index 1f6c803..c755e39 100644
--- a/port.c
+++ b/port.c
@@ -1712,13 +1712,15 @@ int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
return 0;
break;
case COMMAND:
- case RESPONSE:
- case ACKNOWLEDGE:
+ break;
+ default:
return -1;
}

switch (mgt->id) {
+ case NULL_MANAGEMENT:
case CLOCK_DESCRIPTION:
+ case PORT_DATA_SET:
case LOG_ANNOUNCE_INTERVAL:
case ANNOUNCE_RECEIPT_TIMEOUT:
case LOG_SYNC_INTERVAL:
--
1.8.1.2
Geoff Salmon
2013-02-04 18:05:41 UTC
Permalink
It's especially important to check that SET messages aren't empty.

Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/clock.c b/clock.c
index 04a769b..2aa065d 100644
--- a/clock.c
+++ b/clock.c
@@ -649,14 +649,26 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)

switch (management_action(msg)) {
case GET:
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
if (clock_management_get_response(c, p, mgt->id, msg))
return;
break;
case SET:
+ if (mgt->length == 2 && mgt->id != NULL_MANAGEMENT) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
if (clock_management_set(c, p, mgt->id, msg))
return;
break;
case COMMAND:
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
break;
default:
return;
--
1.8.1.2
Keller, Jacob E
2013-02-04 18:30:07 UTC
Permalink
-----Original Message-----
Sent: Monday, February 04, 2013 10:06 AM
Subject: [Linuxptp-devel] [PATCH 6/6] send errors if mgmt tlv length
doesn't match action
It's especially important to check that SET messages aren't empty.
---
clock.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/clock.c b/clock.c
index 04a769b..2aa065d 100644
--- a/clock.c
+++ b/clock.c
@@ -649,14 +649,26 @@ void clock_manage(struct clock *c, struct port
*p, struct ptp_message *msg)
switch (management_action(msg)) {
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg,
WRONG_LENGTH);
+ return;
+ }
if (clock_management_get_response(c, p, mgt->id,
msg))
return;
break;
+ if (mgt->length == 2 && mgt->id != NULL_MANAGEMENT)
{
What's the reason that Set is == 2 here, but the other cases are != 2? Just looks like a typo, but I don't really know for sure if it's incorrect. It seems like since the other two match that this one might also have meant to match..?

- Jake
+ clock_management_send_error(p, msg,
WRONG_LENGTH);
+ return;
+ }
if (clock_management_set(c, p, mgt->id, msg))
return;
break;
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg,
WRONG_LENGTH);
+ return;
+ }
break;
return;
--
1.8.1.2
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
http://p.sf.net/sfu/appdyn_d2d_jan
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Geoff Salmon
2013-02-04 18:39:48 UTC
Permalink
Post by Keller, Jacob E
Post by Geoff Salmon
switch (management_action(msg)) {
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg,
WRONG_LENGTH);
+ return;
+ }
if (clock_management_get_response(c, p, mgt->id,
msg))
return;
break;
+ if (mgt->length == 2 && mgt->id != NULL_MANAGEMENT)
{
What's the reason that Set is == 2 here, but the other cases are != 2? Just looks like a typo, but I don't really know for sure if it's incorrect. It seems like since the other two match that this one might also have meant to match..?
It's checking that the TLV for GET and CMD messages has length == 2 to
ensure they do not have a body. For SETs, all TLVs (except for
NULL_MANAGEMENT) should have a body, so length != 2.

The code in tlv.c allows TLVs received to either be their valid lengths
or be length == 2. In clock_manage, we now know the message's action and
can verify whether or not the TLV should have a body.

Is there a clearer way to express it, or do you think it needs a
comment? I could replace 2 by sizeof(mgt->id), as is done in tlv.c, but
I'm not sure that would help.

- Geoff
Richard Cochran
2013-02-04 18:46:36 UTC
Permalink
Post by Geoff Salmon
It's checking that the TLV for GET and CMD messages has length == 2 to
ensure they do not have a body. For SETs, all TLVs (except for
NULL_MANAGEMENT) should have a body, so length != 2.
But shouldn't we check for the exact length, according to the ID of
the SET? And at this point, since we don't yet accept *any* SET
actions at all, why do we need to check at all?

Thanks,
Richard
Geoff Salmon
2013-02-04 19:07:42 UTC
Permalink
Post by Richard Cochran
Post by Geoff Salmon
It's checking that the TLV for GET and CMD messages has length == 2 to
ensure they do not have a body. For SETs, all TLVs (except for
NULL_MANAGEMENT) should have a body, so length != 2.
But shouldn't we check for the exact length, according to the ID of
the SET? And at this point, since we don't yet accept *any* SET
actions at all, why do we need to check at all?
The exact length according to the ID is checked in tlv.c, but management
TLVs with empty bodies are received to allow GETs and CMDs. So at this
point in clock_manage the TLV either has the correct length or length 2.

I can't argue we need the check before handling SETs. All I can say is
supporting SETs seems inevitable and the added clock_management_set and
port_management_set functions matches the existing code structure. I can
move them to later patch sets if you prefer.

- Geoff
Richard Cochran
2013-02-06 13:51:40 UTC
Permalink
Post by Geoff Salmon
The exact length according to the ID is checked in tlv.c, but
management TLVs with empty bodies are received to allow GETs and
CMDs. So at this point in clock_manage the TLV either has the
correct length or length 2.
This paragraph would make a nice comment for the code in clock_manage().
Post by Geoff Salmon
I can't argue we need the check before handling SETs. All I can say
is supporting SETs seems inevitable and the added
clock_management_set and port_management_set functions matches the
existing code structure. I can move them to later patch sets if you
prefer.
No, I think what you have done is okay. Thanks for explaining.

Richard
Keller, Jacob E
2013-02-06 22:11:39 UTC
Permalink
-----Original Message-----
Sent: Wednesday, February 06, 2013 5:52 AM
To: Geoff Salmon
Subject: Re: [Linuxptp-devel] [PATCH 6/6] send errors if mgmt tlv length
doesn't match action
Post by Geoff Salmon
The exact length according to the ID is checked in tlv.c, but
management TLVs with empty bodies are received to allow GETs and
CMDs. So at this point in clock_manage the TLV either has the
correct length or length 2.
This paragraph would make a nice comment for the code in
clock_manage().
Post by Geoff Salmon
I can't argue we need the check before handling SETs. All I can say
is supporting SETs seems inevitable and the added
clock_management_set and port_management_set functions matches
the
Post by Geoff Salmon
existing code structure. I can move them to later patch sets if you
prefer.
No, I think what you have done is okay. Thanks for explaining.
Richard
Yes it makes sense now that I know. I was just concerned when I initially saw three things that looked the same and wanted to be sure what was different. Thanks
Keller, Jacob E
2013-02-04 18:48:55 UTC
Permalink
-----Original Message-----
Sent: Monday, February 04, 2013 10:40 AM
To: Keller, Jacob E
Subject: Re: [Linuxptp-devel] [PATCH 6/6] send errors if mgmt tlv length
doesn't match action
Post by Keller, Jacob E
Post by Geoff Salmon
switch (management_action(msg)) {
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg,
WRONG_LENGTH);
+ return;
+ }
if (clock_management_get_response(c, p, mgt->id,
msg))
return;
break;
+ if (mgt->length == 2 && mgt->id != NULL_MANAGEMENT)
{
What's the reason that Set is == 2 here, but the other cases are != 2?
Just looks like a typo, but I don't really know for sure if it's incorrect. It
seems like since the other two match that this one might also have
meant to match..?
It's checking that the TLV for GET and CMD messages has length == 2 to
ensure they do not have a body. For SETs, all TLVs (except for
NULL_MANAGEMENT) should have a body, so length != 2.
The code in tlv.c allows TLVs received to either be their valid lengths
or be length == 2. In clock_manage, we now know the message's action and
can verify whether or not the TLV should have a body.
Is there a clearer way to express it, or do you think it needs a
comment? I could replace 2 by sizeof(mgt->id), as is done in tlv.c, but
I'm not sure that would help.
- Geoff
That makes sense. I just wanted to make sure there was a fundamental difference in the messages that made the check different. A comment might be helpful but I'm not sure it is necessary.

- Jake
Geoff Salmon
2013-02-06 16:24:35 UTC
Permalink
It's especially important to check that SET messages aren't empty.

Signed-off-by: Geoff Salmon <***@se-instruments.com>
---
clock.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/clock.c b/clock.c
index ca66bf6..dc93bdc 100644
--- a/clock.c
+++ b/clock.c
@@ -645,16 +645,34 @@ void clock_manage(struct clock *c, struct port *p, struct ptp_message *msg)
}
mgt = (struct management_tlv *) msg->management.suffix;

+ /*
+ The correct length according to the management ID is checked
+ in tlv.c, but management TLVs with empty bodies are also
+ received successfully to support GETs and CMDs. At this
+ point the TLV either has the correct length or length 2.
+ */
switch (management_action(msg)) {
case GET:
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
if (clock_management_get_response(c, p, mgt->id, msg))
return;
break;
case SET:
+ if (mgt->length == 2 && mgt->id != NULL_MANAGEMENT) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
if (clock_management_set(c, p, mgt->id, msg))
return;
break;
case COMMAND:
+ if (mgt->length != 2) {
+ clock_management_send_error(p, msg, WRONG_LENGTH);
+ return;
+ }
break;
default:
return;
--
1.8.1.2
Geoff Salmon
2013-02-06 16:29:10 UTC
Permalink
Hi
I just sent updated versions of patches 4 and 6 which fix the unused
variable warnings and add the comment about why the TLV length might be
2. I think those 2 updated patches address everything. Let me know if I
missed something.

- Geoff
This patch set sends more error messages for invalid management
messages. I've tried to minimize what's changed and only send error
messages in places where the code already detects the error or add new
tests only when necessary (ex: checking that a SET message isn't
empty). For TLVs that have invalid lengths, that is still detected in
tlv.c and the whole message is ignored before clock_manage is even
called.
The 4th patch also adds clock/port_management_set functions to prepare
for supporting the SET messages. I included the patch in this set
because adding the functions affects the way NOT_SUPPORTed errors
should be sent.
fixes typo port_managment_error -> port_management_error
factors out functions for sending mgmt errors from clock and port
send UNKNOWN_ID error for unknown management TLVs
adds clock/port_management_set functions
send NOT_SUPPORTED errors for all unhandled, known management IDs
send errors if mgmt tlv length doesn't match action
clock.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
port.c | 41 +++++++++++++++++++++++++++++++----------
port.h | 4 ++--
3 files changed, 75 insertions(+), 19 deletions(-)
Richard Cochran
2013-02-06 18:15:40 UTC
Permalink
Post by Geoff Salmon
Hi
I just sent updated versions of patches 4 and 6 which fix the unused
variable warnings and add the comment about why the TLV length might be
2. I think those 2 updated patches address everything. Let me know if I
missed something.
I just went through this series again, and it looks pretty good to
me. However, I still have a question on #4 which I will post in a
minute or two.

BTW If you make any changes to this or another patch series, could you
please just post the entire series again, with an updated subject
line? [1] It makes it easier for me than updating individual patches.

Thanks,
Richard

[1] for example: git format-patch --subject-prefix="PATCH v4"
Richard Cochran
2013-02-06 18:22:31 UTC
Permalink
Post by Richard Cochran
I just went through this series again, and it looks pretty good to
me. However, I still have a question on #4 which I will post in a
minute or two.
Never mind about that. I answered my own question. I'll apply this
whole series.

Thanks,
Richard
Loading...