DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
From: "Yigit, Ferruh" <ferruh.yigit@linux.intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>, jgrajcia@cisco.com
Cc: dev@dpdk.org, Konstantin Ananyev <konstantin.ananyev@intel.com>
Subject: Re: [dpdk-dev] [RFC v3] net/memif: allow for full key size in socket name
Date: Fri, 4 Oct 2019 13:41:33 +0100
Message-ID: <f3ce918e-3624-79d5-9e96-e5cb9510b03d@linux.intel.com> (raw)
In-Reply-To: <20190716172057.21110-1-stephen@networkplumber.org>

On 7/16/2019 6:20 PM, Stephen Hemminger wrote:
> The key size for memif is 256 but the unix domain socket structure has
> space for 100 bytes. Change it to use a larger buffer and not hard
> code the keysize everywhere.
> 
> Not sure what purpose of socket is anyway since there is no code
> which connects to it in the current tree anyway?
> 
> Still an RFC, have no way to test.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v3 - fix checkpatch issues
> 
>  drivers/net/memif/memif_socket.c | 29 ++++++++++++++++++-----------
>  drivers/net/memif/memif_socket.h |  4 +++-
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
> index 01a935f87c9f..5eecbdc98040 100644
> --- a/drivers/net/memif/memif_socket.c
> +++ b/drivers/net/memif/memif_socket.c
> @@ -860,11 +860,16 @@ memif_listener_handler(void *arg)
>  		rte_free(cc);
>  }
>  
> +#define MEMIF_SOCKET_UN_SIZE	\
> +	(offsetof(struct sockaddr_un, sun_path) + MEMIF_SOCKET_KEY_LEN)
> +
>  static struct memif_socket *
> -memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
> +memif_socket_create(struct pmd_internals *pmd,
> +		    const char *key, uint8_t listener)
>  {
>  	struct memif_socket *sock;
> -	struct sockaddr_un un;
> +	struct sockaddr_un *un;
> +	char un_buf[MEMIF_SOCKET_UN_SIZE];
>  	int sockfd;
>  	int ret;
>  	int on = 1;
> @@ -876,7 +881,7 @@ memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
>  	}
>  
>  	sock->listener = listener;
> -	rte_memcpy(sock->filename, key, 256);
> +	strlcpy(sock->filename, key, MEMIF_SOCKET_KEY_LEN);
>  	TAILQ_INIT(&sock->dev_queue);
>  
>  	if (listener != 0) {
> @@ -884,15 +889,16 @@ memif_socket_create(struct pmd_internals *pmd, char *key, uint8_t listener)
>  		if (sockfd < 0)
>  			goto error;
>  
> -		un.sun_family = AF_UNIX;
> -		memcpy(un.sun_path, sock->filename,
> -			sizeof(un.sun_path) - 1);
> +		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);
>  
>  		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
>  				 sizeof(on));
>  		if (ret < 0)
>  			goto error;
> -		ret = bind(sockfd, (struct sockaddr *)&un, sizeof(un));
> +		ret = bind(sockfd, (struct sockaddr *)un, MEMIF_SOCKET_UN_SIZE);

Hi Jakub,

While testing your zero-copy patch [1], I stuck to a bind() error [2].
When provided a socket length bigger than "sizeof(struct sockaddr)", bind()
fails. I am testing this on a Fedora system.
I wonder if there is a check in glibc related to the length.

What was your test platform for the change?



[1]
https://patches.dpdk.org/patch/57817/

[2]
memif_socket_create(): NULL: Failed to setup socket /run/memif.sock: Invalid
argument

  parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-08 16:06 [dpdk-dev] [RFC] " Stephen Hemminger
2019-07-16 17:18 ` [dpdk-dev] [RFC v2] " Stephen Hemminger
2019-07-16 17:20 ` [dpdk-dev] [RFC v3] " Stephen Hemminger
2019-08-30  7:17   ` Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)
2019-09-13 18:47     ` Ferruh Yigit
2019-10-04 12:41   ` Yigit, Ferruh [this message]
2019-10-07  9:01     ` Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)
2019-10-07 15:21       ` Ferruh Yigit

Reply instructions:

You may reply publically 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=f3ce918e-3624-79d5-9e96-e5cb9510b03d@linux.intel.com \
    --to=ferruh.yigit@linux.intel.com \
    --cc=dev@dpdk.org \
    --cc=jgrajcia@cisco.com \
    --cc=konstantin.ananyev@intel.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

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git