DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)"
	<jgrajcia@cisco.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v5] net/memif: zero-copy slave
Date: Thu, 17 Oct 2019 17:45:01 +0100	[thread overview]
Message-ID: <782b6d35-b1c4-71c3-afd0-96116b364b3d@intel.com> (raw)
In-Reply-To: <95cc597b-8369-54de-9407-add5b2306f48@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 ...
> 


  reply	other threads:[~2019-10-17 16:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09  8:22 [dpdk-dev] [PATCH v4] " Jakub Grajciar
2019-07-10 15:06 ` Ferruh Yigit
2019-07-23 12:35   ` Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)
2019-08-22  8:18 ` [dpdk-dev] [PATCH v5] " Jakub Grajciar
2019-10-04 13:23   ` Ferruh Yigit
2019-10-15 16:59     ` Ferruh Yigit
2019-10-17 11:52       ` Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)
2019-10-17 16:04         ` Ferruh Yigit
2019-10-17 16:45           ` Ferruh Yigit [this message]
2019-10-21 16:07             ` Ferruh Yigit
2019-10-25 16:44   ` Yigit, Ferruh
2019-10-29 14:28     ` David Marchand
2019-10-30 10:17       ` Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)
2019-10-30 10:25         ` David Marchand
2019-11-04 11:03   ` [dpdk-dev] [PATCH v6] " Jakub Grajciar
2019-11-11 15:21     ` Ferruh Yigit
2019-11-11 15:24       ` Thomas Monjalon
2019-11-12 12:55         ` Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)
2019-11-11 15:49       ` David Marchand
2019-11-15 16:55         ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=782b6d35-b1c4-71c3-afd0-96116b364b3d@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=jgrajcia@cisco.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).