[Linuxptp-devel] [PATCH] ptp4l: Add a method to check whether the network driver supports software time stamp

[Dong Zhu]

Signed-off-by: Dong Zhu <***@gmail.com>
---
ptp4l.c | 8 ++++++++
sk.c | 39 +++++++++++++++++++++++++++++++++++++++
sk.h | 7 +++++++
3 files changed, 54 insertions(+)

diff --git a/ptp4l.c b/ptp4l.c
index 5838d53..377ada9 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -253,6 +253,14 @@ int main(int argc, char *argv[])
return -1;
}

+ if (*timestamping == TS_SOFTWARE) {
+ if (software_timestamp_support(iface[0].name)) {
+ fprintf(stderr, "network driver do not "
+ "support Software Time Stamp\n");
+ return -1;
+ }
+ }
+
/* determine PHC Clock index */
if (ds->free_running) {
phc_index = -1;
diff --git a/sk.c b/sk.c
index b6ba8a5..7257386 100644
--- a/sk.c
+++ b/sk.c
@@ -123,6 +123,45 @@ int sk_interface_phc(char *name, int *index)
#endif
}

+int software_timestamp_support(char *name)
+{
+#ifdef ETHTOOL_GET_TS_INFO
+ struct ethtool_ts_info info;
+ struct ifreq ifr;
+ int fd, err;
+
+ memset(&ifr, 0, sizeof(ifr));
+ memset(&info, 0, sizeof(info));
+ info.cmd = ETHTOOL_GET_TS_INFO;
+ strcpy(ifr.ifr_name, name);
+ ifr.ifr_data = (char *) &info;
+ fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (fd < 0) {
+ pr_err("socket failed: %m");
+ return -1;
+ }
+
+ err = ioctl(fd, SIOCETHTOOL, &ifr);
+ if (err < 0) {
+ pr_err("ioctl SIOCETHTOOL failed: %m");
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+
+ if (info.so_timestamping & (SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE))
+ return 0;
+ else
+ return -1;
+#else
+ return -1;
+#endif
+}
+
int sk_interface_macaddr(char *name, unsigned char *mac, int len)
{
struct ifreq ifreq;
diff --git a/sk.h b/sk.h
index 671e839..6a4aa38 100644
--- a/sk.h
+++ b/sk.h
@@ -48,6 +48,13 @@ int sk_interface_phc(char *name, int *index);
int sk_interface_macaddr(char *name, unsigned char *mac, int len);

/**
+ * Check whether the network driver support software time stamp
+ * @param name The name of the interface
+ * @return Zero on success, non-zero otherwise.
+ */
+int software_timestamp_support(char *name);
+
+/**
* Read a message from a socket.
* @param fd An open socket.
* @param buf Buffer to receive the message.

[Richard Cochran]

Post by Dong Zhu
---
ptp4l.c | 8 ++++++++
sk.c | 39 +++++++++++++++++++++++++++++++++++++++
sk.h | 7 +++++++
3 files changed, 54 insertions(+)
diff --git a/ptp4l.c b/ptp4l.c
index 5838d53..377ada9 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -253,6 +253,14 @@ int main(int argc, char *argv[])
return -1;
}
+ if (*timestamping == TS_SOFTWARE) {
+ if (software_timestamp_support(iface[0].name)) {
+ fprintf(stderr, "network driver do not "
+ "support Software Time Stamp\n");
+ return -1;
+ }
+ }
+
/* determine PHC Clock index */
if (ds->free_running) {
phc_index = -1;
diff --git a/sk.c b/sk.c
index b6ba8a5..7257386 100644
--- a/sk.c
+++ b/sk.c
@@ -123,6 +123,45 @@ int sk_interface_phc(char *name, int *index)
#endif
}
+int software_timestamp_support(char *name)
+{
+#ifdef ETHTOOL_GET_TS_INFO
+ struct ethtool_ts_info info;
+ struct ifreq ifr;
+ int fd, err;
+
+ memset(&ifr, 0, sizeof(ifr));
+ memset(&info, 0, sizeof(info));
+ info.cmd = ETHTOOL_GET_TS_INFO;
+ strcpy(ifr.ifr_name, name);
+ ifr.ifr_data = (char *) &info;
+ fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (fd < 0) {
+ pr_err("socket failed: %m");
+ return -1;
+ }
+
+ err = ioctl(fd, SIOCETHTOOL, &ifr);
+ if (err < 0) {
+ pr_err("ioctl SIOCETHTOOL failed: %m");
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+
+ if (info.so_timestamping & (SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE))
+ return 0;
+ else
+ return -1;
+#else
+ return -1;

If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

Post by Dong Zhu
+#endif
+}
+
int sk_interface_macaddr(char *name, unsigned char *mac, int len)
{
struct ifreq ifreq;
diff --git a/sk.h b/sk.h
index 671e839..6a4aa38 100644
--- a/sk.h
+++ b/sk.h
@@ -48,6 +48,13 @@ int sk_interface_phc(char *name, int *index);
int sk_interface_macaddr(char *name, unsigned char *mac, int len);
/**
+ * Check whether the network driver support software time stamp
+ */
+int software_timestamp_support(char *name);

It would be nice to have this function's name begin with sk_ prefix,
like the others.

Maybe "sk_swts_support"?

Thanks,
Richard

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, November 13, 2012 3:56 AM
To: Dong Zhu
Subject: Re: [Linuxptp-devel] [PATCH] ptp4l: Add a method to check whether
the network driver supports software time stamp

Post by Dong Zhu
---
ptp4l.c | 8 ++++++++
sk.c | 39 +++++++++++++++++++++++++++++++++++++++
sk.h | 7 +++++++
3 files changed, 54 insertions(+)
diff --git a/ptp4l.c b/ptp4l.c
index 5838d53..377ada9 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -253,6 +253,14 @@ int main(int argc, char *argv[])
return -1;
}

<snip>

Post by Dong Zhu
+ if (info.so_timestamping & (SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE))
+ return 0;
+ else
+ return -1;
+#else
+ return -1;

If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

Agreed.

Post by Dong Zhu
+#endif
+}
+
int sk_interface_macaddr(char *name, unsigned char *mac, int len)
{
struct ifreq ifreq;
diff --git a/sk.h b/sk.h
index 671e839..6a4aa38 100644
--- a/sk.h
+++ b/sk.h
@@ -48,6 +48,13 @@ int sk_interface_phc(char *name, int *index);
int sk_interface_macaddr(char *name, unsigned char *mac, int len);
/**
+ * Check whether the network driver support software time stamp
+ */
+int software_timestamp_support(char *name);

It would be nice to have this function's name begin with sk_ prefix,
like the others.
Maybe "sk_swts_support"?

It should match sk_ format, and personally I would make it more general, by
passing the type of time stamping you want to check? Do we have a function
in place for checking rx filter already?

Thanks

- Jake

Thanks,
Richard
--------------------------------------------------------------------------
----
Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
Linuxptp-devel mailing list
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Richard Cochran]

Post by Keller, Jacob E
It should match sk_ format, and personally I would make it more general, by
passing the type of time stamping you want to check? Do we have a function
in place for checking rx filter already?

We already have sk_interface_phc to check the PHC index. This function
might as well check the supported time stamping types too.

Thanks,
Richard

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, November 13, 2012 10:49 AM
To: Keller, Jacob E
Subject: Re: [Linuxptp-devel] [PATCH] ptp4l: Add a method to check whether
the network driver supports software time stamp

Post by Keller, Jacob E
It should match sk_ format, and personally I would make it more general,

by

Post by Keller, Jacob E
passing the type of time stamping you want to check? Do we have a

function

Post by Keller, Jacob E
in place for checking rx filter already?

We already have sk_interface_phc to check the PHC index. This function
might as well check the supported time stamping types too.

Ok I'll write up a patch to expand support.

Thanks

- Jake

Thanks,
Richard

[Dong Zhu]

Post by Richard Cochran
If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

agreed,have modified the patch.

Post by Richard Cochran
It would be nice to have this function's name begin with sk_ prefix,
like the others.
Maybe "sk_swts_support"?

Agreed,As you know the sk_interface_phc function used to check the PHC index.
So when I composed this patch I also considered to modify this function(such as add a new option)
to check the supported time stamping types,too.But I think this will
cause large changes to the code,so I prefer to write a new function to
implement it.

"sk_swts_support" this name is OK.

Modified Patch:


Signed-off-by: Dong Zhu <***@gmail.com>
---
ptp4l.c | 14 ++++++++++++++
sk.c | 39 +++++++++++++++++++++++++++++++++++++++
sk.h | 7 +++++++
3 files changed, 60 insertions(+)

diff --git a/ptp4l.c b/ptp4l.c
index 5838d53..a16abc4 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -253,6 +253,20 @@ int main(int argc, char *argv[])
return -1;
}

+ if (*timestamping == TS_SOFTWARE) {
+ int value;
+
+ if ((value = sk_swts_support(iface[0].name))) {
+ if (value == 1)
+ fprintf(stderr, "network driver do not "
+ "support Software Time Stamp\n");
+ else
+ fprintf(stderr, "get_ts_info not supported\n");
+
+ return -1;
+ }
+ }
+
/* determine PHC Clock index */
if (ds->free_running) {
phc_index = -1;
diff --git a/sk.c b/sk.c
index b6ba8a5..a3d3427 100644
--- a/sk.c
+++ b/sk.c
@@ -123,6 +123,45 @@ int sk_interface_phc(char *name, int *index)
#endif
}

+int sk_swts_support(char *name)
+{
+#ifdef ETHTOOL_GET_TS_INFO
+ struct ethtool_ts_info info;
+ struct ifreq ifr;
+ int fd, err;
+
+ memset(&ifr, 0, sizeof(ifr));
+ memset(&info, 0, sizeof(info));
+ info.cmd = ETHTOOL_GET_TS_INFO;
+ strcpy(ifr.ifr_name, name);
+ ifr.ifr_data = (char *) &info;
+ fd = socket(AF_INET, SOCK_DGRAM, 0);
+
+ if (fd < 0) {
+ pr_err("socket failed: %m");
+ return -1;
+ }
+
+ err = ioctl(fd, SIOCETHTOOL, &ifr);
+ if (err < 0) {
+ pr_err("ioctl SIOCETHTOOL failed: %m");
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+
+ if (info.so_timestamping & (SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE))
+ return 0;
+ else
+ return 1;
+#else
+ return -1;
+#endif
+}
+
int sk_interface_macaddr(char *name, unsigned char *mac, int len)
{
struct ifreq ifreq;
diff --git a/sk.h b/sk.h
index 671e839..9806841 100644
--- a/sk.h
+++ b/sk.h
@@ -48,6 +48,13 @@ int sk_interface_phc(char *name, int *index);
int sk_interface_macaddr(char *name, unsigned char *mac, int len);

/**
+ * Check whether the network driver support software time stamp
+ * @param name The name of the interface
+ * @return Zero on success, non-zero otherwise.
+ */
+int sk_swts_support(char *name);
+
+/**
* Read a message from a socket.
* @param fd An open socket.
* @param buf Buffer to receive the message.

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, November 13, 2012 7:22 PM
To: Richard Cochran
Subject: Re: [Linuxptp-devel] [PATCH] ptp4l: Add a method to check whether
the network driver supports software time stamp

Post by Richard Cochran
If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

agreed,have modified the patch.

Post by Richard Cochran
It would be nice to have this function's name begin with sk_ prefix,
like the others.
Maybe "sk_swts_support"?

Agreed,As you know the sk_interface_phc function used to check the PHC index.
So when I composed this patch I also considered to modify this
function(such as add a new option)
to check the supported time stamping types,too.But I think this will
cause large changes to the code,so I prefer to write a new function to
implement it.

I sent an RFC which changed sk_interface_phc to sk_get_ts_info. Take a look
at that, and we can decide what makes the best sense.

Thanks

- Jake

[Richard Cochran]

Post by Dong Zhu

Post by Richard Cochran
If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

agreed,have modified the patch.

Post by Richard Cochran
It would be nice to have this function's name begin with sk_ prefix,
like the others.
Maybe "sk_swts_support"?

Agreed,As you know the sk_interface_phc function used to check the PHC index.
So when I composed this patch I also considered to modify this function(such as add a new option)
to check the supported time stamping types,too.But I think this will
cause large changes to the code,so I prefer to write a new function to
implement it.
"sk_swts_support" this name is OK.

At this point I prefer Jacob's patches, although there is nothing
really wrong with yours.

Thanks anyhow for the idea and the patch,

Richard

[Dong Zhu]

Post by Richard Cochran

Post by Dong Zhu

Post by Richard Cochran
If compiled without ETHTOOL_GET_TS_INFO support, shouldn't we assume
that the user knows what he is doing, and return zero (=okay) here?

agreed,have modified the patch.

Post by Richard Cochran
It would be nice to have this function's name begin with sk_ prefix,
like the others.
Maybe "sk_swts_support"?

Agreed,As you know the sk_interface_phc function used to check the PHC index.
So when I composed this patch I also considered to modify this function(such as add a new option)
to check the supported time stamping types,too.But I think this will
cause large changes to the code,so I prefer to write a new function to
implement it.
"sk_swts_support" this name is OK.

At this point I prefer Jacob's patches, although there is nothing
really wrong with yours.
Thanks anyhow for the idea and the patch,
Richard

Never Mind, Jacob's patches look very nice ~~