On 10/17/2019 5:45 PM, Ferruh Yigit wrote:
> 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: step...@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?
Ping.
Can it possible to make this on time so rc1 doesn't released with broken PMD?
Thanks,
ferruh
>
>
>
> 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 ...
>>
>
