Message ID | 20170107005542.GA16278@felix.cavium.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
From: Felix Manlunas <felix.manlunas@cavium.com> Date: Fri, 6 Jan 2017 16:55:42 -0800 > > + if (rh->r_dh.has_hash) { > + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); Is the checksum defined to be in the first 4-bytes of the 8-byte DHLEN unit, or the second 4-bytes? Is the answer to this question endian-dependent?
David Miller <davem@davemloft.net> wrote on Sun [2017-Jan-08 17:09:41 -0500]: > From: Felix Manlunas <felix.manlunas@cavium.com> > Date: Fri, 6 Jan 2017 16:55:42 -0800 > > > > > + if (rh->r_dh.has_hash) { > > + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); > > Is the checksum defined to be in the first 4-bytes of the 8-byte DHLEN unit, > or the second 4-bytes? Is the answer to this question endian-dependent? The hash is always in the first 4-bytes. The location of the hash is endian independent. The hash itself (in its original form) is big endian.
On Fri, 2017-01-06 at 16:55 -0800, Felix Manlunas wrote: > From: Prasad Kanneganti <prasad.kanneganti@cavium.com> > > Store the L4 hash of received packets in the skb; the hash is computed in > the NIC firmware. ... > > + if (rh->r_dh.has_hash) { > + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); sparse should complain on this line, right ? make C=2 CF=-D__CHECK_ENDIAN__ M=drivers/net/ethernet/cavium
From: Felix Manlunas <felix.manlunas@cavium.com> Date: Mon, 9 Jan 2017 10:45:05 -0800 > David Miller <davem@davemloft.net> wrote on Sun [2017-Jan-08 17:09:41 -0500]: >> From: Felix Manlunas <felix.manlunas@cavium.com> >> Date: Fri, 6 Jan 2017 16:55:42 -0800 >> >> > >> > + if (rh->r_dh.has_hash) { >> > + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); >> >> Is the checksum defined to be in the first 4-bytes of the 8-byte DHLEN unit, >> or the second 4-bytes? Is the answer to this question endian-dependent? > > The hash is always in the first 4-bytes. The location of the hash is endian > independent. The hash itself (in its original form) is big endian. Thanks for explaining. Please fix the SPARSE issue Eric Dumazet mentioned.
David Miller <davem@davemloft.net> wrote on Mon [2017-Jan-09 13:56:21 -0500]: > From: Felix Manlunas <felix.manlunas@cavium.com> > Date: Mon, 9 Jan 2017 10:45:05 -0800 > > > David Miller <davem@davemloft.net> wrote on Sun [2017-Jan-08 17:09:41 -0500]: > >> From: Felix Manlunas <felix.manlunas@cavium.com> > >> Date: Fri, 6 Jan 2017 16:55:42 -0800 > >> > >> > > >> > + if (rh->r_dh.has_hash) { > >> > + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); > >> > >> Is the checksum defined to be in the first 4-bytes of the 8-byte DHLEN unit, > >> or the second 4-bytes? Is the answer to this question endian-dependent? > > > > The hash is always in the first 4-bytes. The location of the hash is endian > > independent. The hash itself (in its original form) is big endian. > > Thanks for explaining. > > Please fix the SPARSE issue Eric Dumazet mentioned. Will do. Thanks.
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index 39a9665..adae858 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -2263,6 +2263,7 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), struct skb_shared_hwtstamps *shhwtstamps; u64 ns; u16 vtag = 0; + u32 r_dh_off; struct net_device *netdev = (struct net_device *)arg; struct octeon_droq *droq = container_of(param, struct octeon_droq, napi); @@ -2308,6 +2309,8 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), put_page(pg_info->page); } + r_dh_off = (rh->r_dh.len - 1) * BYTES_PER_DHLEN_UNIT; + if (((oct->chip_id == OCTEON_CN66XX) || (oct->chip_id == OCTEON_CN68XX)) && ptp_enable) { @@ -2320,16 +2323,26 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), /* Nanoseconds are in the first 64-bits * of the packet. */ - memcpy(&ns, (skb->data), sizeof(ns)); + memcpy(&ns, (skb->data + r_dh_off), + sizeof(ns)); + r_dh_off -= BYTES_PER_DHLEN_UNIT; shhwtstamps = skb_hwtstamps(skb); shhwtstamps->hwtstamp = ns_to_ktime(ns + lio->ptp_adjust); } - skb_pull(skb, sizeof(ns)); } } + if (rh->r_dh.has_hash) { + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); + + skb_set_hash(skb, hash, PKT_HASH_TYPE_L4); + r_dh_off -= BYTES_PER_DHLEN_UNIT; + } + + skb_pull(skb, rh->r_dh.len * BYTES_PER_DHLEN_UNIT); + skb->protocol = eth_type_trans(skb, skb->dev); if ((netdev->features & NETIF_F_RXCSUM) && (((rh->r_dh.encap_on) && diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c index 70d96c1..c0ee746 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c @@ -1497,6 +1497,7 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), struct net_device *netdev = (struct net_device *)arg; struct sk_buff *skb = (struct sk_buff *)skbuff; u16 vtag = 0; + u32 r_dh_off; if (netdev) { struct lio *lio = GET_LIO(netdev); @@ -1540,7 +1541,19 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), put_page(pg_info->page); } - skb_pull(skb, rh->r_dh.len * 8); + r_dh_off = (rh->r_dh.len - 1) * BYTES_PER_DHLEN_UNIT; + + if (rh->r_dh.has_hwtstamp) + r_dh_off -= BYTES_PER_DHLEN_UNIT; + + if (rh->r_dh.has_hash) { + u32 hash = be32_to_cpu(*(u32 *)(skb->data + r_dh_off)); + + skb_set_hash(skb, hash, PKT_HASH_TYPE_L4); + r_dh_off -= BYTES_PER_DHLEN_UNIT; + } + + skb_pull(skb, rh->r_dh.len * BYTES_PER_DHLEN_UNIT); skb->protocol = eth_type_trans(skb, skb->dev); if ((netdev->features & NETIF_F_RXCSUM) && diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h index ba329f6..bc0af8a 100644 --- a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h +++ b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h @@ -98,6 +98,8 @@ enum octeon_tag_type { #define CVM_DRV_INVALID_APP (CVM_DRV_APP_START + 0x2) #define CVM_DRV_APP_END (CVM_DRV_INVALID_APP - 1) +#define BYTES_PER_DHLEN_UNIT 8 + static inline u32 incr_index(u32 index, u32 count, u32 max) { if ((index + count) >= max)