packet: move hw/sw timestamp extraction into a small helper
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 packet: move hw/sw timestamp extraction into a small helper
This patch introduces a small helper function, that is used by AF_PACKET. Based on the flags that are passed, it extracts the packet timestamp in the receive path. This is merely a refactoring to remove some duplicate code in tpacket_rcv(), make it more readable, and to enable others to use this function as well.
--- include/linux/skbuff.h | 20 ++++++++++++++++++++ net/packet/af_packet.c | 38 ++++---------------------------------- 2 files changed, 24 insertions(+), 34 deletions(-)
include/linux/skbuff.h +20 -0
@@ -33,6 +33,7 @@#include <linux/dma-mapping.h> #include <linux/netdev_features.h> #include <net/flow_keys.h> +#include <linux/net_tstamp.h> /* Don't change this without changing skb_csum_unnecessary! */ #define CHECKSUM_NONE 0
@@ -739,6 +740,25 @@ static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb)return &skb_shinfo (skb)->hwtstamps; } +static inline struct timespec skb_get_tstamp (struct sk_buff *skb, +unsigned int flags) +{ +struct skb_shared_hwtstamps *ts =skb_hwtstamps (skb); + +if ((flags &SOF_TIMESTAMPING_SYS_HARDWARE ) && ts->syststamp.tv64) +return ktime_to_timespec (ts->syststamp); +else if ((flags &SOF_TIMESTAMPING_RAW_HARDWARE ) && ts->hwtstamp.tv64) +return ktime_to_timespec (ts->hwtstamp); +else if (skb->tstamp.tv64) +return ktime_to_timespec (skb->tstamp); +else { +struct timespec tc; + +getnstimeofday (&tc); +return tc; + } +} +/** * skb_queue_empty - check if a queue is empty * @list: queue head
net/packet/af_packet.c +4 -34
@@ -1681,9 +1681,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,unsigned long status =TP_STATUS_USER ;unsigned short macoff, netoff, hdrlen;struct sk_buff *copy_skb =NULL ; -struct timeval tv;struct timespec ts; -struct skb_shared_hwtstamps *shhwtstamps =skb_hwtstamps (skb);if (skb->pkt_type ==PACKET_LOOPBACK ) goto drop;
@@ -1767,24 +1765,16 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,skb_copy_bits (skb,0 , h.raw + macoff, snaplen); + ts =skb_get_tstamp (skb, po->tp_tstamp); +switch (po->tp_version) {case TPACKET_V1 : h.h1->tp_len = skb->len; h.h1->tp_snaplen = snaplen; h.h1->tp_mac = macoff; h.h1->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - tv =ktime_to_timeval (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - tv =ktime_to_timeval (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - tv =ktime_to_timeval (skb->tstamp); -else -do_gettimeofday (&tv); - h.h1->tp_sec = tv.tv_sec; - h.h1->tp_usec = tv.tv_usec; + h.h1->tp_sec = ts.tv_sec; + h.h1->tp_usec = ts.tv_nsec /NSEC_PER_USEC ; hdrlen =sizeof (*h.h1);break ;case TPACKET_V2 :
@@ -1792,16 +1782,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_snaplen = snaplen; h.h2->tp_mac = macoff; h.h2->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h2->tp_sec = ts.tv_sec; h.h2->tp_nsec = ts.tv_nsec;if (vlan_tx_tag_present (skb)) {
@@ -1822,16 +1802,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h3->tp_snaplen = snaplen; h.h3->tp_mac = macoff; h.h3->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h3->tp_sec = ts.tv_sec; h.h3->tp_nsec = ts.tv_nsec; hdrlen =sizeof (*h.h3);
Signature · 1 line
1.7.11.7
Quoted 6 lines · Daniel Borkmann
This patch introduces a small helper function, that is used by
AF_PACKET. Based on the flags that are passed, it extracts the packet timestamp in the receive path. This is merely a refactoring to remove some duplicate code intpacket_rcv(), make it more readable, and to enable others to use this function as well.
Quoted 6 lines
Cc Willem de Bruijn <[email protected]>Signed-off-by Daniel Borkmann <[email protected]>---
include/linux/skbuff.h| 20 ++++++++++++++++++++net/packet/af_packet.c| 38 ++++----------------------------------2 files changed, 24 insertions(+), 34 deletions(-)
Nice consolidation, good work Daniel :-)
Quoted 5 lines · Daniel Borkmann
This patch introduces a small helper function, that is used by
AF_PACKET. Based on the flags that are passed, it extracts the packet timestamp in the receive path. This is merely a refactoring to remove some duplicate code intpacket_rcv(), make it more readable, and to enable others to use this function as well.
Good idea. I noticed that while browsing the code over the weekend, too.
One question I have is whether the code warrants adding a static inline to skbuff.h. Regular socket paths do not use the test, as they return all timestamps together.
Quoted 20 lines
Cc Willem de Bruijn <[email protected]>Signed-off-by Daniel Borkmann <[email protected]>--- include/linux/skbuff.h | 20 ++++++++++++++++++++ net/packet/af_packet.c | 38 ++++---------------------------------- 2 files changed, 24 insertions(+), 34 deletions(-)include/linux/skbuff.h +1
-0@@ -33,6 +33,7 @@#include <linux/dma-mapping.h> #include <linux/netdev_features.h> #include <net/flow_keys.h> +#include <linux/net_tstamp.h> /* Don't change this without changing skb_csum_unnecessary! */ #define CHECKSUM_NONE 0
include/linux/skbuff.h static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb) · line 748
Quoted 9 lines
include/linux/skbuff.h +6
-0@@ -739,6 +740,25 @@ static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb)return &skb_shinfo (skb)->hwtstamps; } +static inline struct timespec skb_get_tstamp (struct sk_buff *skb, +unsigned int flags) +{ +struct skb_shared_hwtstamps *ts =skb_hwtstamps (skb); + +if ((flags &SOF_TIMESTAMPING_SYS_HARDWARE ) && ts->syststamp.tv64)
Also, packet sockets use the flags these SOF_... constants passed from userspace directly, but regular socket paths convert this into socket flags SOCK_..., so this code is more packet socket specific than it seems. See __sock_recv_timestamp. I'd keep the function in net/packet/af_packet.c solely for this reason, or add a comment.
Quoted 96 lines
include/linux/skbuff.h +13
-0+return ktime_to_timespec (ts->syststamp); + else if ((flags & SOF_TIMESTAMPING_RAW_HARDWARE) && ts->hwtstamp.tv64) +return ktime_to_timespec (ts->hwtstamp); + else if (skb->tstamp.tv64) + return ktime_to_timespec(skb->tstamp); + else { +struct timespec tc; + +getnstimeofday (&tc); +return tc; + } +} +/** * skb_queue_empty - check if a queue is empty * @list: queue head net/packet/af_packet.c +4
-35@@ -1681,9 +1681,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,unsigned long status =TP_STATUS_USER ;unsigned short macoff, netoff, hdrlen;struct sk_buff *copy_skb =NULL ; -struct timeval tv;struct timespec ts; -struct skb_shared_hwtstamps *shhwtstamps =skb_hwtstamps (skb);if (skb->pkt_type ==PACKET_LOOPBACK ) goto drop;@@ -1767,24 +1765,16 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,skb_copy_bits (skb,0 , h.raw + macoff, snaplen); + ts =skb_get_tstamp (skb, po->tp_tstamp); +switch (po->tp_version) {case TPACKET_V1 : h.h1->tp_len = skb->len; h.h1->tp_snaplen = snaplen; h.h1->tp_mac = macoff; h.h1->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - tv =ktime_to_timeval (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - tv =ktime_to_timeval (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - tv =ktime_to_timeval (skb->tstamp); -else -do_gettimeofday (&tv); - h.h1->tp_sec = tv.tv_sec; - h.h1->tp_usec = tv.tv_usec; + h.h1->tp_sec = ts.tv_sec; + h.h1->tp_usec = ts.tv_nsec /NSEC_PER_USEC ; hdrlen =sizeof (*h.h1);break ;case TPACKET_V2 :@@ -1792,16 +1782,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_snaplen = snaplen; h.h2->tp_mac = macoff; h.h2->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h2->tp_sec = ts.tv_sec; h.h2->tp_nsec = ts.tv_nsec;if (vlan_tx_tag_present (skb)) {@@ -1822,16 +1802,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h3->tp_snaplen = snaplen; h.h3->tp_mac = macoff; h.h3->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h3->tp_sec = ts.tv_sec; h.h3->tp_nsec = ts.tv_nsec; hdrlen =sizeof (*h.h3); --1.7.11.7
Quoted 7 lines · Willem de Bruijn
Quoted 5 lines · Daniel Borkmann
This patch introduces a small helper function, that is used by
AF_PACKET. Based on the flags that are passed, it extracts the packet timestamp in the receive path. This is merely a refactoring to remove some duplicate code intpacket_rcv(), make it more readable, and to enable others to use this function as well.
Quoted 5 lines
Good idea. I noticed that while browsing the code over the weekend, too.
One question I have is whether the code warrants adding a static inline to
skbuff.h. Regular socket paths do not use the test, as they return all timestamps together.
(See below.)
Quoted 23 lines
Quoted 23 lines
Cc Willem de Bruijn <[email protected]>Signed-off-by Daniel Borkmann <[email protected]>--- include/linux/skbuff.h | 20 ++++++++++++++++++++ net/packet/af_packet.c | 38 ++++---------------------------------- 2 files changed, 24 insertions(+), 34 deletions(-)include/linux/skbuff.h +1
-0@@ -33,6 +33,7 @@#include <linux/dma-mapping.h> #include <linux/netdev_features.h> #include <net/flow_keys.h> +#include <linux/net_tstamp.h> /* Don't change this without changing skb_csum_unnecessary! */ #define CHECKSUM_NONE 0@@ -739,6 +740,25 @@ static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb)return &skb_shinfo (skb)->hwtstamps; }
Quoted 12 lines
include/linux/skbuff.h static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb) · line 748
Quoted 6 lines
include/linux/skbuff.h +6
-0+static inline struct timespec skb_get_tstamp (struct sk_buff *skb, +unsigned int flags) +{ +struct skb_shared_hwtstamps *ts =skb_hwtstamps (skb); + +if ((flags &SOF_TIMESTAMPING_SYS_HARDWARE ) && ts->syststamp.tv64)Also, packet sockets use the flags these
SOF_... constants passed from userspace directly, but regular socket paths convert this into socket flagsSOCK_..., so this code is more packet socket specific than it seems. See__sock_recv_timestamp. I'd keep the function innet/packet/af_packet.csolely for this reason, or add a comment.
Hm, right. I do not really see a reason why we need to maintain this twice (introduced by 614f60fa9 ``packet_mmap: expose hw packet timestamps to network packet capture utilities'').
This could also have been implemented actually with normal SOCK_ flags, i.e. since we use sock_tx_timestamp() which uses SOCK_ flags. I will respin and try to address that.
Quoted 96 lines
Quoted 96 lines
include/linux/skbuff.h +13
-0+return ktime_to_timespec (ts->syststamp); + else if ((flags & SOF_TIMESTAMPING_RAW_HARDWARE) && ts->hwtstamp.tv64) +return ktime_to_timespec (ts->hwtstamp); + else if (skb->tstamp.tv64) + return ktime_to_timespec(skb->tstamp); + else { +struct timespec tc; + +getnstimeofday (&tc); +return tc; + } +} +/** * skb_queue_empty - check if a queue is empty * @list: queue head net/packet/af_packet.c +4
-35@@ -1681,9 +1681,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,unsigned long status =TP_STATUS_USER ;unsigned short macoff, netoff, hdrlen;struct sk_buff *copy_skb =NULL ; -struct timeval tv;struct timespec ts; -struct skb_shared_hwtstamps *shhwtstamps =skb_hwtstamps (skb);if (skb->pkt_type ==PACKET_LOOPBACK ) goto drop;@@ -1767,24 +1765,16 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,skb_copy_bits (skb,0 , h.raw + macoff, snaplen); + ts =skb_get_tstamp (skb, po->tp_tstamp); +switch (po->tp_version) {case TPACKET_V1 : h.h1->tp_len = skb->len; h.h1->tp_snaplen = snaplen; h.h1->tp_mac = macoff; h.h1->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - tv =ktime_to_timeval (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - tv =ktime_to_timeval (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - tv =ktime_to_timeval (skb->tstamp); -else -do_gettimeofday (&tv); - h.h1->tp_sec = tv.tv_sec; - h.h1->tp_usec = tv.tv_usec; + h.h1->tp_sec = ts.tv_sec; + h.h1->tp_usec = ts.tv_nsec /NSEC_PER_USEC ; hdrlen =sizeof (*h.h1);break ;case TPACKET_V2 :@@ -1792,16 +1782,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h2->tp_snaplen = snaplen; h.h2->tp_mac = macoff; h.h2->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h2->tp_sec = ts.tv_sec; h.h2->tp_nsec = ts.tv_nsec;if (vlan_tx_tag_present (skb)) {@@ -1822,16 +1802,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, h.h3->tp_snaplen = snaplen; h.h3->tp_mac = macoff; h.h3->tp_net = netoff; -if ((po->tp_tstamp &SOF_TIMESTAMPING_SYS_HARDWARE ) - && shhwtstamps->syststamp.tv64) - ts =ktime_to_timespec (shhwtstamps->syststamp); -else if ((po->tp_tstamp &SOF_TIMESTAMPING_RAW_HARDWARE ) - && shhwtstamps->hwtstamp.tv64) - ts =ktime_to_timespec (shhwtstamps->hwtstamp); -else if (skb->tstamp.tv64) - ts =ktime_to_timespec (skb->tstamp); -else -getnstimeofday (&ts); h.h3->tp_sec = ts.tv_sec; h.h3->tp_nsec = ts.tv_nsec; hdrlen =sizeof (*h.h3); --1.7.11.7