On Mon, Mar 24, 2014 at 09:53:22AM +0100, Jiri Benc wrote:
> This puts groundwork for event subscription and notification. The individual
> events are added by subsequent patches.
This patch is basically okay, but I have to pick a few nits...
> +struct clock_subscriber {
> + LIST_ENTRY(clock_subscriber) list;
> + unsigned int events;
> + struct PortIdentity targetPortIdentity;
> + uint8_t addr[TRANSPORT_RECV_ADDR_LEN];
> + int addrlen;
> + UInteger16 sequenceId;
> +};
This code looks good, and I think it will be able support unicast
subscriptions too, later on.
> diff --git a/tlv.c b/tlv.c
> index b8cdd3959c9b..352026ee5f7a 100644
> --- a/tlv.c
> +++ b/tlv.c
> @@ -242,6 +242,16 @@ static int mgt_post_recv(struct management_tlv *m,
> uint16_t data_len,
> pdsnp->neighborPropDelayThresh =
> ntohl(pdsnp->neighborPropDelayThresh);
> pdsnp->asCapable = ntohl(pdsnp->asCapable);
> break;
> + case SUBSCRIBE_EVENTS_NP:
> + {
> + struct subscribe_events_np *sen;
Please, no braces in a case.
Put 'sen' at the top with the others.
> +
> + if (data_len != sizeof(struct subscribe_events_np))
> + goto bad_length;
> + sen = (struct subscribe_events_np *)m->data;
> + sen->bitmask = ntohl(sen->bitmask);
> + }
> + break;
> case SAVE_IN_NON_VOLATILE_STORAGE:
> case RESET_NON_VOLATILE_STORAGE:
> case INITIALIZE:
> @@ -337,6 +347,14 @@ static void mgt_pre_send(struct management_tlv *m,
> struct tlv_extra *extra)
> pdsnp->neighborPropDelayThresh =
> htonl(pdsnp->neighborPropDelayThresh);
> pdsnp->asCapable = htonl(pdsnp->asCapable);
> break;
> + case SUBSCRIBE_EVENTS_NP:
> + {
> + struct subscribe_events_np *sen;
> +
> + sen = (struct subscribe_events_np *)m->data;
> + sen->bitmask = htonl(sen->bitmask);
> + }
> + break;
> }
> }
>
> diff --git a/tlv.h b/tlv.h
> index 60c937db02ef..8eb840685a81 100644
> --- a/tlv.h
> +++ b/tlv.h
> @@ -79,6 +79,7 @@ enum management_action {
> #define PRIMARY_DOMAIN 0x4002
> #define TIME_STATUS_NP 0xC000
> #define GRANDMASTER_SETTINGS_NP 0xC001
> +#define SUBSCRIBE_EVENTS_NP 0xC003
>
> /* Port management ID values */
> #define NULL_MANAGEMENT 0x0000
> @@ -196,6 +197,10 @@ struct port_ds_np {
> Integer32 asCapable;
> } PACKED;
>
> +struct subscribe_events_np {
> + UInteger32 bitmask;
Please use c99 types instead (unless a field has a type like that in
some public standard). We hate camel case.
Also, I anticipate that we will want many more than 32 events. Lets
add a whole bunch of space here for future use.
Thanks,
Richard
> +} PACKED;
> +
> enum clock_type {
> CLOCK_TYPE_ORDINARY = 0x8000,
> CLOCK_TYPE_BOUNDARY = 0x4000,
> --
> 1.7.6.5
------------------------------------------------------------------------------
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
