Re: [bug report] net/mlx5e: TLS, Add Innova TLS TX offload data path

Re: [bug report] net/mlx5e: TLS, Add Innova TLS TX offload data path

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

+ Saeed, Boris and netdev

On Thu, May 03, 2018 at 02:23:00PM +0300, Dan Carpenter wrote:
> Hello Ilya Lesokhin,
>
> The patch bf23974104fa: "net/mlx5e: TLS, Add Innova TLS TX offload
> data path" from Apr 30, 2018, leads to the following static checker
> warning:
>
> 	drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls_rxtx.c:63 mlx5e_tls_add_metadata()
> 	warn: struct type mismatch 'ethhdr vs mlx5e_tls_metadata'
>
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls_rxtx.c
>     55  static int mlx5e_tls_add_metadata(struct sk_buff *skb, __be32 swid)
>     56  {
>     57          struct mlx5e_tls_metadata *pet;
>     58          struct ethhdr *eth;
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     59
>     60          if (skb_cow_head(skb, sizeof(struct mlx5e_tls_metadata)))
>                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     61                  return -ENOMEM;
>     62
>     63          eth = (struct ethhdr *)skb_push(skb, sizeof(struct mlx5e_tls_metadata));
>                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This feels like it should be sizeof(*eth) + sizeof(*pet)?
>
>     64          skb->mac_header -= sizeof(struct mlx5e_tls_metadata);
>     65          pet = (struct mlx5e_tls_metadata *)(eth + 1);
>     66
>     67          memmove(skb->data, skb->data + sizeof(struct mlx5e_tls_metadata),
>     68                  2 * ETH_ALEN);
>     69
>     70          eth->h_proto = cpu_to_be16(MLX5E_METADATA_ETHER_TYPE);
>     71          pet->syndrome_swid = htonl(SYNDROME_OFFLOAD_REQUIRED << 24) | swid;
>     72
>     73          return 0;
>     74  }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [bug report] net/mlx5e: TLS, Add Innova TLS TX offload data path

[Boris Pismenny]

Hi Dan,

Thanks for taking a look at our code.

On 05/03/18 15:07, Leon Romanovsky wrote:
> + Saeed, Boris and netdev
>
> On Thu, May 03, 2018 at 02:23:00PM +0300, Dan Carpenter wrote:
>> Hello Ilya Lesokhin,
>>
>> The patch bf23974104fa: "net/mlx5e: TLS, Add Innova TLS TX offload
>> data path" from Apr 30, 2018, leads to the following static checker
>> warning:
>>
>> 	drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls_rxtx.c:63 mlx5e_tls_add_metadata()
>> 	warn: struct type mismatch 'ethhdr vs mlx5e_tls_metadata'
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en_accel/tls_rxtx.c
>>      55  static int mlx5e_tls_add_metadata(struct sk_buff *skb, __be32 swid)
>>      56  {
>>      57          struct mlx5e_tls_metadata *pet;
>>      58          struct ethhdr *eth;
>>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      59
>>      60          if (skb_cow_head(skb, sizeof(struct mlx5e_tls_metadata)))
>>                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      61                  return -ENOMEM;
>>      62
>>      63          eth = (struct ethhdr *)skb_push(skb, sizeof(struct mlx5e_tls_metadata));
>>                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> This feels like it should be sizeof(*eth) + sizeof(*pet)?
No, we add only "sizeof(struct mlx5e_tls_metadata)" bytes to the skb 
headroom for the mlx5_tls_metadata. The Ethernet header is already there.
>>
>>      64          skb->mac_header -= sizeof(struct mlx5e_tls_metadata);
>>      65          pet = (struct mlx5e_tls_metadata *)(eth + 1);
>>      66
>>      67          memmove(skb->data, skb->data + sizeof(struct mlx5e_tls_metadata),
>>      68                  2 * ETH_ALEN);
>>      69
>>      70          eth->h_proto = cpu_to_be16(MLX5E_METADATA_ETHER_TYPE);
>>      71          pet->syndrome_swid = htonl(SYNDROME_OFFLOAD_REQUIRED << 24) | swid;
>>      72
>>      73          return 0;
>>      74  }
>>
>> regards,
>> dan carpenter
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Best,
Boris.