On Thu, 29 Dec 2022 at 07:52, SyncMonk Technologies <servi...@syncmonk.net>
wrote:
> Get the interface speed related information using ethtool and
> convert interface speed to bit rate in attoseconds per bit.
>
> v4: adding interface bit period in iface_if_info. This bit period
> is updated during init time and whenever there are port events
> like link up and down and speed updates.
>
> Signed-off-by: Greg Armstrong <greg.armstrong...@renesas.com>
> Signed-off-by: Leon Goldin <leon.goldin...@renesas.com>
> Signed-off-by: Devasish Dey <devasish....@syncmonk.net>
> Signed-off-by: Vipin Sharma <vipin.sha...@syncmonk.net>
> ---
> clock.c | 6 ++++--
> interface.c | 12 ++++++++++++
> interface.h | 14 ++++++++++++++
> port.c | 5 +++++
> sk.c | 2 ++
> sk.h | 2 ++
> 6 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/clock.c b/clock.c
> index 134c7c3..94da41b 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1005,11 +1005,13 @@ struct clock *clock_create(enum clock_type type,
> struct config *config,
> memset(ts_label, 0, sizeof(ts_label));
> if (!rtnl_get_ts_device(interface_name(iface), ts_label))
> interface_set_label(iface, ts_label);
> + /* Interface speed information */
> + interface_get_ifinfo(iface);
> interface_get_tsinfo(iface);
> if (interface_tsinfo_valid(iface) &&
> - !interface_tsmodes_supported(iface, required_modes)) {
> + !interface_tsmodes_supported(iface,
> required_modes)) {
>
Do you use Linux kernel alignment?
Why is that line shift?
> pr_err("interface '%s' does not support requested
> timestamping mode",
> - interface_name(iface));
> + interface_name(iface));
> return NULL;
> }
> }
> diff --git a/interface.c b/interface.c
> index 6c2630c..8524719 100644
> --- a/interface.c
> +++ b/interface.c
> @@ -12,6 +12,7 @@ struct interface {
> char name[MAX_IFNAME_SIZE + 1];
> char ts_label[MAX_IFNAME_SIZE + 1];
> struct sk_ts_info ts_info;
> + struct sk_if_info if_info;
> int vclock;
> };
>
> @@ -40,11 +41,22 @@ int interface_get_tsinfo(struct interface *iface)
> return sk_get_ts_info(iface->ts_label, &iface->ts_info);
> }
>
> +int interface_get_ifinfo(struct interface *iface)
> +{
> + return sk_get_if_info(iface->ts_label, &iface->if_info);
> +}
> +
> const char *interface_label(struct interface *iface)
> {
> return iface->ts_label;
> }
>
> +bool interface_ifinfo_valid(struct interface *iface)
> +{
> + return iface->if_info.valid ? true : false;
>
Boolean does not need to be translated to boolean.
You can simply "valid != 0", depending on 'valid' value.
Another one popular in Linux kernel is "!!valid".
> +}
> +
> +
> const char *interface_name(struct interface *iface)
> {
> return iface->name;
> diff --git a/interface.h b/interface.h
> index 5fc7836..5289a7f 100644
> --- a/interface.h
> +++ b/interface.h
> @@ -40,6 +40,13 @@ void interface_destroy(struct interface *iface);
> */
> int interface_get_tsinfo(struct interface *iface);
>
> +/**
> + * Populate the time stamping information of a given interface.
> + * @param iface The interface of interest.
> + * @return zero on success, negative on failure.
> + */
> +int interface_get_ifinfo(struct interface *iface);
> +
> /**
> * Obtain the time stamping label of a network interface. This can be
> * different from the name of the interface when bonding is in effect.
> @@ -77,6 +84,13 @@ void interface_set_label(struct interface *iface, const
> char *label);
> */
> bool interface_tsinfo_valid(struct interface *iface);
>
> +/**
> + * Tests whether an interface's interface information is valid or not.
> + * @param iface The interface of interest.
> + * @return True if the interface information is valid, false
> otherwise.
> + */
> +bool interface_ifinfo_valid(struct interface *iface);
> +
> /**
> * Tests whether an interface supports a set of given time stamping modes.
> * @param iface The interface of interest.
> diff --git a/port.c b/port.c
> index 6baf5c8..2a96c40 100644
> --- a/port.c
> +++ b/port.c
> @@ -2741,6 +2741,11 @@ void port_link_status(void *ctx, int linkup, int
> ts_index)
> p->link_status = link_state;
> } else {
> p->link_status = link_state | LINK_STATE_CHANGED;
> + /* Update Interface speed information on Link up*/
> + if (linkup) {
> + interface_get_ifinfo(p->iface);
> + }
> +
> pr_notice("%s: link %s", p->log_name, linkup ? "up" :
> "down");
> }
>
> diff --git a/sk.c b/sk.c
> index 1c14ca3..2e4ef2c 100644
> --- a/sk.c
> +++ b/sk.c
> @@ -267,6 +267,8 @@ int sk_get_if_info(const char *name, struct sk_if_info
> *if_info)
> if_info->valid = 1;
>
Perhaps using "valid = true" and "valid = false"
So in interface_ifinfo_valid() you can simply return the value without any
conversion.
> if_info->speed = ecmd.req.speed;
>
> + /* Megabits per second converted to attoseconds per bit */
+ if_info->iface_bit_period = (1000000000000ULL/if_info->speed);
>
Excellent place for division!
> return 0;
> failed:
> #endif
> diff --git a/sk.h b/sk.h
> index 7a9058a..df105d6 100644
> --- a/sk.h
> +++ b/sk.h
> @@ -54,10 +54,12 @@ struct sk_ts_info {
> * Contains interface information returned by the GLINKSETTINGS ioctl.
> * @valid: set to non-zero when the info struct contains valid
> data.
> * @speed: interface speed.
> + * @iface_bit_period interface bit period in attoseconds per bit.
>
Please specify the ITU standard that recommends using attoseconds.
*/
> struct sk_if_info {
> bool valid;
> uint32_t speed;
> + uint64_t iface_bit_period;
> };
>
> /**
> --
> 2.34.1
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
