Re: [dpdk-dev] [PATCH v5] net/memif: zero-copy slave

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/17/2019 5:04 PM, Ferruh Yigit wrote:
> On 10/17/2019 12:52 PM, Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at
> Cisco) wrote:
>>
>>> Hi Jakub,
>>>
>>> Just to double check if anyone is looking into the bind() error issue which is
>>> since following commit, I am waiting for more input on it.
>>>
>>> Commit b923866c6974 ("net/memif: allow for full key size in socket name")
>>> Cc: stephen@networkplumber.org
>>
>> Definitely an issue, I must have messed something up while applying the patch for testing, as it's not working for me either. I looked into bind() and it seems that the size has upper limit. Are you able to revert the patch?
> 
> +1 to logically revert the patch, but actually I think it is easier to do an
> incremental patch on top of current head to fix the issue.
> 
> The patch replaces hard-coded 'key_len' to a macro which is good to keep, also
> number of lines needs to be changed for fix looks less than number of lines will
>  be changed by revert J
> 
>>
>> As for the issue that patch addresses, there is an incorrect information in the docs that 'socket' parameter length is 256b. It should be 108b as that is the limitation on Linux.
> 
> I don't know the doc but code has it as 256b [1] and the issue patch addresses
> looks like a valid issue. If you are OK to limit the 'key_len' to 108 [2], the
> fix is really easy.

Hi Jakub,

Something like following seems fixing the issue, can you please make a patch for
the fix?



 diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
 index 0c71f6c454..aa4f549f2c 100644
 --- a/drivers/net/memif/memif_socket.c
 +++ b/drivers/net/memif/memif_socket.c
 @@ -868,8 +868,7 @@ memif_socket_create(struct pmd_internals *pmd,
                     const char *key, uint8_t listener)
  {
         struct memif_socket *sock;
 -       struct sockaddr_un *un;
 -       char un_buf[MEMIF_SOCKET_UN_SIZE];
 +       struct sockaddr_un un;
         int sockfd;
         int ret;
         int on = 1;
 @@ -889,16 +888,14 @@ memif_socket_create(struct pmd_internals *pmd,
                 if (sockfd < 0)
                         goto error;

 -               memset(un_buf, 0, sizeof(un_buf));
 -               un = (struct sockaddr_un *)un_buf;
 -               un->sun_family = AF_UNIX;
 -               strlcpy(un->sun_path, sock->filename, MEMIF_SOCKET_KEY_LEN);
 +               un.sun_family = AF_UNIX;
 +               strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_KEY_LEN);

                 ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
                                  sizeof(on));
                 if (ret < 0)
                         goto error;
 -               ret = bind(sockfd, (struct sockaddr *)un,  MEMIF_SOCKET_UN_SIZE);
 +               ret = bind(sockfd, (struct sockaddr *)&un, sizeof(struct
sockaddr_un));
                 if (ret < 0)
                         goto error;
                 ret = listen(sockfd, 1);
 diff --git a/drivers/net/memif/memif_socket.h b/drivers/net/memif /memif_socket.h
 index 9f40f8d138..12fa123230 100644
 --- a/drivers/net/memif/memif_socket.h
 +++ b/drivers/net/memif/memif_socket.h
 @@ -79,7 +79,7 @@ struct memif_socket_dev_list_elt {
  };

  #define MEMIF_SOCKET_HASH_NAME                 "memif-sh"
 -#define MEMIF_SOCKET_KEY_LEN           256
 +#define MEMIF_SOCKET_KEY_LEN           100

  struct memif_socket {
         struct rte_intr_handle intr_handle;     /**< interrupt handle */



> 
> 
> [1]
> http://lxr.dpdk.org/dpdk/v19.08/source/drivers/net/memif/memif_socket.h#L84
> 
> [2]
> If I remember correctly the suggested length was 99 for portability, it seems
> different OS has this value different and 99 is the smallest ...
>