On Wed, 2022-12-07 at 17:34 +0530, SyncMonk Technologies wrote:
> When master and slave instance interacting with each other operating
> at different interface speed, delay assymetry needs to be compensated
> as described in G.8271 appendix V.
> 
> In this patch we are adding changes to get the interface speed using
> ethtool.
> 
> v1: initial commit.
> v2: updating comments and data types.
> v3: updating Boolean data type to bool from <stdbool.h>
> ---
>  sk.c | 71
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  sk.h | 19 ++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/sk.c b/sk.c
> index d27abff..1d1a656 100644
> --- a/sk.c
> +++ b/sk.c
> @@ -205,6 +205,77 @@ failed:
>         return -1;
>  }
>  
> +int sk_get_if_info(const char *name, struct sk_if_info *if_info)
> +{
> +#ifdef ETHTOOL_GLINKSETTINGS
> +       struct ifreq ifr;
> +       int fd, err;
> +
> +       struct {
> +               struct ethtool_link_settings req;
> +               /*
> +                * link_mode_data consists of supported[],
> advertising[],
> +                * lp_advertising[] with size up to 127 each.
> +                * The actual size is provided by the kernel.
> +                */
> +               __u32 link_mode_data[3 * 127];
> +       } ecmd;
> +
> +       memset(&ifr, 0, sizeof(ifr));
> +       memset(&ecmd, 0, sizeof(ecmd));
> +
> +       fd = socket(AF_INET, SOCK_DGRAM, 0);
> +       if (fd < 0) {
> +               goto failed;
> +       }
> +
> +       ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
> +
> +       strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
> +       ifr.ifr_data = (char *) &ecmd;
> +
> +       /* Handshake with kernel to determine number of words for
> link
> +        * mode bitmaps. When requested number of bitmap words is not
> +        * the one expected by kernel, the latter returns the integer
> +        * opposite of what it is expecting. We request length 0
> below
> +        * (aka. invalid bitmap length) to get this info.
> +        */
> +       err = ioctl(fd, SIOCETHTOOL, &ifr);
> +       if (err < 0) {
> +               pr_err("ioctl SIOCETHTOOL failed: %m");
> +               close(fd);
> +               goto failed;
> +       }

I think it is better to merge this ioctl and the socket creation with
sk_get_ts_info().
No reason for duplication.
You can use a static function or inline one.

> +
> +       if (ecmd.req.link_mode_masks_nwords >= 0 ||
> +                       ecmd.req.cmd != ETHTOOL_GLINKSETTINGS) {
> +               return 1;
> +       }
> +       ecmd.req.link_mode_masks_nwords = -
> ecmd.req.link_mode_masks_nwords;
> +
> +       err = ioctl(fd, SIOCETHTOOL, &ifr);
> +       if (err < 0) {
> +               pr_err("ioctl SIOCETHTOOL failed: %m");
> +               close(fd);
> +               goto failed;
> +       }
> +
> +       close(fd);
> +
> +       /* copy the necessary data to sk_info */
> +       memset(if_info, 0, sizeof(struct sk_if_info));
> +       if_info->valid = 1;
> +       if_info->speed = ecmd.req.speed;
> +
> +       return 0;
> +failed:
> +#endif
> +       /* clear data and ensure it is not marked valid */
> +       memset(if_info, 0, sizeof(struct sk_if_info));
> +       return -1;

You need to close the socket!

> +}
> +
> +
>  static int sk_interface_guidaddr(const char *name, unsigned char
> *guid)
>  {
>         char file_name[64], buf[64], addr[8];
> diff --git a/sk.h b/sk.h
> index 486dbc4..4cee3d7 100644
> --- a/sk.h
> +++ b/sk.h
> @@ -20,6 +20,7 @@
>  #ifndef HAVE_SK_H
>  #define HAVE_SK_H
>  
> +#include <stdbool.h>
>  #include "address.h"
>  #include "transport.h"
>  
> @@ -49,6 +50,16 @@ struct sk_ts_info {
>         unsigned int rx_filters;
>  };
>  
> +/**
> + * Contains interface information returned by theGLINKSETTINGS
> ioctl.
> + * @valid:            set to non-zero when the info struct contains
> valid data.
> + * @speed:            interface speed.
> + */
> +struct sk_if_info {
> +       bool valid;
It is better not to use bool in a structure, as the size is usually
int, i.e. 64 bits to hold 1 bit.

> +       int speed;
What are the units here?
Old systems use 32 bits, if you measure in bits per second, the maximum
is 4 GB.
It is better to use uint64_t and use 1 mbps as units.

> +};
> +
>  /**
>   * Obtains a socket suitable for use with sk_interface_index().
>   * @return  An open socket on success, -1 otherwise.
> @@ -78,6 +89,14 @@ int sk_general_init(int fd);
>   */
>  int sk_get_ts_info(const char *name, struct sk_ts_info *sk_info);
>  
> +/**
> + * Obtain supporte interface information
> + * @param name     The name of the interface
> + * @param info      Struct containing obtained interface
> information.
> + * @return          zero on success, negative on failure.
> + */
> +int sk_get_if_info(const char *name, struct sk_if_info *sk_info);
> +
>  /**
>   * Obtain the MAC address of a network interface.
>   * @param name  The name of the interface


Erez


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to