[Linuxptp-devel] [PATCH 1/2] pmc: optimize duplicated code in do_set_action()

[Petr Kulhavy]

TLV_PRIORITY1 and TLV_PRIORITY2 cases in do_set_action() use the same repeated
piece of generic code for setting one-value parameter. Remove the duplicated
code and let both cases use the same code.

Signed-off-by: Petr Kulhavy <***@jikos.cz>
---
pmc.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/pmc.c b/pmc.c
index 261f2a2..cefa771 100644
--- a/pmc.c
+++ b/pmc.c
@@ -515,14 +515,6 @@ static void do_set_action(int action, int index, char *str)
}
switch (code) {
case TLV_PRIORITY1:
- cnt = sscanf(str, " %*s %*s %hhu", &mtd.val);
- if (cnt != 1) {
- fprintf(stderr, "%s SET needs 1 value\n",
- idtab[index].name);
- break;
- }
- pmc_send_set_action(pmc, code, &mtd, sizeof(mtd));
- break;
case TLV_PRIORITY2:
cnt = sscanf(str, " %*s %*s %hhu", &mtd.val);
if (cnt != 1) {

[Petr Kulhavy]

This patch enables setting the domainNumber via the pmc command in runtime.

Signed-off-by: Petr Kulhavy <***@jikos.cz>
---
clock.c | 6 ++++++
pmc.c | 3 ++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index 629a160..a2a61a2 100644
--- a/clock.c
+++ b/clock.c
@@ -538,6 +538,12 @@ static int clock_management_set(struct clock *c, struct port *p,
*changed = 1;
respond = 1;
break;
+ case TLV_DOMAIN:
+ mtd = (struct management_tlv_datum *) tlv->data;
+ c->dds.domainNumber = mtd->val;
+ *changed = 1;
+ respond = 1;
+ break;
case TLV_GRANDMASTER_SETTINGS_NP:
gsn = (struct grandmaster_settings_np *) tlv->data;
c->dds.clockQuality = gsn->clockQuality;
diff --git a/pmc.c b/pmc.c
index cefa771..63e33b1 100644
--- a/pmc.c
+++ b/pmc.c
@@ -68,7 +68,7 @@ struct management_id idtab[] = {
{ "TIME_PROPERTIES_DATA_SET", TLV_TIME_PROPERTIES_DATA_SET, do_get_action },
{ "PRIORITY1", TLV_PRIORITY1, do_set_action },
{ "PRIORITY2", TLV_PRIORITY2, do_set_action },
- { "DOMAIN", TLV_DOMAIN, do_get_action },
+ { "DOMAIN", TLV_DOMAIN, do_set_action },
{ "SLAVE_ONLY", TLV_SLAVE_ONLY, do_get_action },
{ "TIME", TLV_TIME, not_supported },
{ "CLOCK_ACCURACY", TLV_CLOCK_ACCURACY, do_get_action },
@@ -516,6 +516,7 @@ static void do_set_action(int action, int index, char *str)
switch (code) {
case TLV_PRIORITY1:
case TLV_PRIORITY2:
+ case TLV_DOMAIN:
cnt = sscanf(str, " %*s %*s %hhu", &mtd.val);
if (cnt != 1) {
fprintf(stderr, "%s SET needs 1 value\n",

[Miroslav Lichvar]

Post by Petr Kulhavy
This patch enables setting the domainNumber via the pmc command in runtime.
--- a/clock.c
+++ b/clock.c
@@ -538,6 +538,12 @@ static int clock_management_set(struct clock *c, struct port *p,
*changed = 1;
respond = 1;
break;
+ mtd = (struct management_tlv_datum *) tlv->data;
+ c->dds.domainNumber = mtd->val;
+ *changed = 1;
+ respond = 1;
+ break;

Shouldn't this also reset the state of the clock?

[Richard Cochran]

Post by Miroslav Lichvar

Post by Petr Kulhavy
+ mtd = (struct management_tlv_datum *) tlv->data;
+ c->dds.domainNumber = mtd->val;
+ *changed = 1;
+ respond = 1;
+ break;

Shouldn't this also reset the state of the clock?

Yes, each port of the clock would have to start again in
PS_INITIALIZING and listen for announce message in the new domain.

Alternatively, you could let the ports keep track of multiple domains
in parallel, allowing more seamless domain switching, but that of
course is much more work.

[ The 'changed' flag only causes a state decision event, but that
would be wrong here because the change in domain invalidates the
ports' foreign master records. ]

Thanks,
Richard

[Richard Cochran]

Post by Miroslav Lichvar
Shouldn't this also reset the state of the clock?

Thinking out loud, the cleanest way might be to introduce a new fault
type called "reconfiguration" with a default timeout of ASAP, and then
throw the fault if the domain changes.

Thanks,
Richard

[Richard Cochran]

Post by Petr Kulhavy
TLV_PRIORITY1 and TLV_PRIORITY2 cases in do_set_action() use the same repeated
piece of generic code for setting one-value parameter. Remove the duplicated
code and let both cases use the same code.

Applied.

Thanks,
Richard