Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Or Gerlitz
On Thu, Aug 6, 2015 at 2:23 AM, Hefty, Sean sean.he...@intel.com wrote: I don't think we should over-complex things vs. what the network stack does for many (since kernel 2.4?!) years. They have basically three flags NETIF_F_IP_CSUM - device can checksum TCP/UDP over IPv4 NETIF_F_IP6_CSUM -

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Haggai Eran
On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Wed, Aug 05, 2015 at 06:34:26PM +0300, Amir Vadai wrote: struct ib_uverbs_ex_query_device { __u32 comp_mask; + __u32 csum_caps; __u32 reserved; }; Uh no. This is the struct

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Parav Pandit
On Thu, Aug 6, 2015 at 4:30 PM, Haggai Eran hagg...@mellanox.com wrote: On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Wed, Aug 05, 2015 at 06:34:26PM +0300, Amir Vadai wrote: struct ib_uverbs_ex_query_device { __u32 comp_mask; +

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Haggai Eran
On 08/06/2015 02:18 PM, Parav Pandit wrote: On Thu, Aug 6, 2015 at 4:30 PM, Haggai Eran hagg...@mellanox.com mailto:hagg...@mellanox.com wrote: On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe jguntho...@obsidianresearch.com mailto:jguntho...@obsidianresearch.com wrote:

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Parav Pandit
On Thu, Aug 6, 2015 at 10:20 PM, Haggai Eran hagg...@mellanox.com wrote: On 08/06/2015 02:18 PM, Parav Pandit wrote: On Thu, Aug 6, 2015 at 4:30 PM, Haggai Eran hagg...@mellanox.com mailto:hagg...@mellanox.com wrote: On Wednesday, August 5, 2015 8:16 PM, Jason Gunthorpe

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-06 Thread Or Gerlitz
On Thu, Aug 6, 2015 at 3:00 AM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: [...] The participating we are sorely lacking right now is on the review side, which is like most of the kernel, unfortunately. I agree, if a proper internal review was taking place here, it wouldn't been

RE: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Hefty, Sean
+enum ib_csum_cap_flags { + IB_CSUM_RX_TCP_UDP = 1 0, + IB_CSUM_RX_IP_HDR= 1 1, + IB_CSUM_TX_TCP_UDP = 1 2, + IB_CSUM_TX_IP_HDR= 1 3 +}; TPC and UDP should be separate flags. -- To unsubscribe from this list: send the line unsubscribe

RE: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Hefty, Sean
+enum ib_csum_cap_flags { + IB_CSUM_RX_TCP_UDP = 1 0, + IB_CSUM_RX_IP_HDR= 1 1, + IB_CSUM_TX_TCP_UDP = 1 2, + IB_CSUM_TX_IP_HDR= 1 3 +}; TPC and UDP should be separate flags. Can you explain why? For the same reason that you didn't include

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Jason Gunthorpe
On Wed, Aug 05, 2015 at 06:34:26PM +0300, Amir Vadai wrote: struct ib_uverbs_ex_query_device { __u32 comp_mask; + __u32 csum_caps; __u32 reserved; }; Uh no. @@ -221,6 +222,7 @@ struct ib_uverbs_odp_caps { struct ib_uverbs_ex_query_device_resp { struct

RE: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Yevgeny Petrilin
+enum ib_csum_cap_flags { + IB_CSUM_RX_TCP_UDP = 1 0, + IB_CSUM_RX_IP_HDR= 1 1, + IB_CSUM_TX_TCP_UDP = 1 2, + IB_CSUM_TX_IP_HDR= 1 3 +}; TPC and UDP should be separate flags. Can you explain why? What we are advertising here is offloads for

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Or Gerlitz
On Wed, Aug 5, 2015 at 8:16 PM, Jason Gunthorpe jguntho...@obsidianresearch.com wrote: On Wed, Aug 05, 2015 at 06:34:26PM +0300, Amir Vadai wrote: struct ib_uverbs_ex_query_device { __u32 comp_mask; + __u32 csum_caps; __u32 reserved; }; Uh no. @@ -221,6 +222,7 @@ struct

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Jason Gunthorpe
On Thu, Aug 06, 2015 at 01:16:17AM +0300, Or Gerlitz wrote: So -- shit happens, I am trying to figure out if an internal review has been done, b/c we do have some folks who terribly master the extended uverbs framework, right...? You and Matan had no problem doing the timestamp stuff properly

Re: [PATCH for-next 1/2] IB/core: Add support for RX/TX checksum offload capabilities report

2015-08-05 Thread Or Gerlitz
On Wed, Aug 5, 2015 at 7:17 PM, Hefty, Sean sean.he...@intel.com wrote: TPC and UDP should be separate flags. Sean, I don't think we should over-complex things vs. what the network stack does for many (since kernel 2.4?!) years. They have basically three flags NETIF_F_IP_CSUM - device can