[dpdk-dev] [PATCH] net/memif: use abstract socket address

[dpdk-dev] [PATCH] net/memif: use abstract socket address

[Jakub Grajciar]

Abstract socket address has no connection with
filesystem pathnames and the socket dissapears
once all open references are closed.

Memif pmd will use abstract socket address by default.
For backwards compatibility use new argument
'socket-abstract=no'

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
---
 doc/guides/nics/memif.rst         |  1 +
 drivers/net/memif/memif_socket.c  | 25 +++++++++++++++--------
 drivers/net/memif/rte_eth_memif.c | 34 ++++++++++++++++++++++++++++++-
 drivers/net/memif/rte_eth_memif.h | 10 +++++----
 4 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index ddeebed25..8e80105a4 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -43,6 +43,7 @@ client.
    "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
    "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
    "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
+   "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
    "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
    "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
    "zero-copy=yes", "Enable/disable zero-copy slave mode. Only relevant to slave, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 67794cb6f..4b0b12ead 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -862,10 +862,10 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 {
 	struct memif_socket *sock;
-	struct sockaddr_un un;
+	struct sockaddr_un un = { 0 };
 	int sockfd;
 	int ret;
 	int on = 1;
@@ -886,7 +886,12 @@ memif_socket_create(char *key, uint8_t listener)
 			goto error;
 
 		un.sun_family = AF_UNIX;
-		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		if (is_abstract) {
+			// abstract address
+			un.sun_path[0] = '\0';
+			strlcpy(un.sun_path + 1, sock->filename, MEMIF_SOCKET_UN_SIZE - 1);
+		} else
+			strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
 
 		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
 				 sizeof(on));
@@ -963,7 +968,7 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	ret = rte_hash_lookup_data(hash, key, (void **)&socket);
 	if (ret < 0) {
 		socket = memif_socket_create(key,
-					     (pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1);
+					     (pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1, pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
@@ -1025,7 +1030,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 	/* remove socket, if this was the last device using it */
 	if (TAILQ_EMPTY(&socket->dev_queue)) {
 		rte_hash_del_key(hash, socket->filename);
-		if (socket->listener) {
+		if (socket->listener && !(pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
 			/* remove listener socket file,
 			 * so we can create new one later.
 			 */
@@ -1054,7 +1059,7 @@ memif_connect_slave(struct rte_eth_dev *dev)
 {
 	int sockfd;
 	int ret;
-	struct sockaddr_un sun;
+	struct sockaddr_un sun = { 0 };
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	memset(pmd->local_disc_string, 0, ETH_MEMIF_DISC_STRING_SIZE);
@@ -1068,8 +1073,12 @@ memif_connect_slave(struct rte_eth_dev *dev)
 	}
 
 	sun.sun_family = AF_UNIX;
-
-	memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
+	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
+		// abstract address
+		sun.sun_path[0] = '\0';
+		memcpy(sun.sun_path + 1, pmd->socket_filename, sizeof(sun.sun_path) - 2);
+	} else
+		memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
 
 	ret = connect(sockfd, (struct sockaddr *)&sun,
 		      sizeof(struct sockaddr_un));
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index c1c7e9f8d..ba285be54 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -36,6 +36,7 @@
 #define ETH_MEMIF_PKT_BUFFER_SIZE_ARG	"bsize"
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
+#define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -46,6 +47,7 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
+	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1640,7 +1642,23 @@ memif_set_socket_filename(const char *key __rte_unused, const char *value,
 	const char **socket_filename = (const char **)extra_args;
 
 	*socket_filename = value;
-	return memif_check_socket_filename(*socket_filename);
+	return 0;
+}
+
+static int
+memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	uint32_t *flags = (uint32_t *)extra_args;
+
+	if (strstr(value, "yes") != NULL) {
+		*flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else if (strstr(value, "no") != NULL) {
+		*flags &= ~ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else {
+		MIF_LOG(ERR, "Failed to parse socket-abstract param: %s.", value);
+		return -EINVAL;
+	}
+	return 0;
 }
 
 static int
@@ -1726,6 +1744,9 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 		MIF_LOG(WARNING, "Failed to register mp action callback: %s",
 			strerror(rte_errno));
 
+	// use abstract address by default
+	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
 
 	/* parse parameters */
@@ -1751,6 +1772,10 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 (void *)(&socket_filename));
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+					 &memif_set_is_socket_abstract, &flags);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1765,6 +1790,12 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 			goto exit;
 	}
 
+	if (!(flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
+		ret = memif_check_socket_filename(socket_filename);
+		if (ret < 0)	
+			goto exit;
+	}
+
 	/* create interface */
 	ret = memif_create(vdev, role, id, flags, socket_filename,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
@@ -1802,6 +1833,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
+				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 6f45b7072..ac0a2f2d1 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -81,14 +81,16 @@ struct pmd_internals {
 	memif_interface_id_t id;		/**< unique id */
 	enum memif_role_t role;			/**< device role */
 	uint32_t flags;				/**< device status flags */
-#define ETH_MEMIF_FLAG_CONNECTING	(1 << 0)
+#define ETH_MEMIF_FLAG_CONNECTING		(1 << 0)
 /**< device is connecting */
-#define ETH_MEMIF_FLAG_CONNECTED	(1 << 1)
+#define ETH_MEMIF_FLAG_CONNECTED		(1 << 1)
 /**< device is connected */
-#define ETH_MEMIF_FLAG_ZERO_COPY	(1 << 2)
+#define ETH_MEMIF_FLAG_ZERO_COPY		(1 << 2)
 /**< device is zero-copy enabled */
-#define ETH_MEMIF_FLAG_DISABLED		(1 << 3)
+#define ETH_MEMIF_FLAG_DISABLED			(1 << 3)
 /**< device has not been configured and can not accept connection requests */
+#define ETH_MEMIF_FLAG_SOCKET_ABSTRACT	(1 << 4)
+/**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
-- 
2.17.1

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Ferruh Yigit]

On 10/5/2020 1:39 PM, Jakub Grajciar wrote:
> Abstract socket address has no connection with
> filesystem pathnames and the socket dissapears
> once all open references are closed.
> 
> Memif pmd will use abstract socket address by default.
> For backwards compatibility use new argument
> 'socket-abstract=no'
> 

Why this backward compatibility is required? How the end user affected from 
swithching to abstract sockets?
Since when linux supports abstract sockets, does this switch will cause problem 
with old kernel versions?

Is there any benefit of the abstract sockets other than socket cleaned 
automatically (I assume for unix sockets it is done when file filesystem 
reference removed)?

> Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
> ---
>   doc/guides/nics/memif.rst         |  1 +
>   drivers/net/memif/memif_socket.c  | 25 +++++++++++++++--------
>   drivers/net/memif/rte_eth_memif.c | 34 ++++++++++++++++++++++++++++++-
>   drivers/net/memif/rte_eth_memif.h | 10 +++++----
>   4 files changed, 57 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
> index ddeebed25..8e80105a4 100644
> --- a/doc/guides/nics/memif.rst
> +++ b/doc/guides/nics/memif.rst
> @@ -43,6 +43,7 @@ client.
>      "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
>      "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
>      "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
> +   "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
>      "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
>      "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
>      "zero-copy=yes", "Enable/disable zero-copy slave mode. Only relevant to slave, requires '--single-file-segments' eal argument", "no", "yes|no"
> diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
> index 67794cb6f..4b0b12ead 100644
> --- a/drivers/net/memif/memif_socket.c
> +++ b/drivers/net/memif/memif_socket.c
> @@ -862,10 +862,10 @@ memif_listener_handler(void *arg)
>   }
>   
>   static struct memif_socket *
> -memif_socket_create(char *key, uint8_t listener)
> +memif_socket_create(char *key, uint8_t listener, bool is_abstract)
>   {
>   	struct memif_socket *sock;
> -	struct sockaddr_un un;
> +	struct sockaddr_un un = { 0 };
>   	int sockfd;
>   	int ret;
>   	int on = 1;
> @@ -886,7 +886,12 @@ memif_socket_create(char *key, uint8_t listener)
>   			goto error;
>   
>   		un.sun_family = AF_UNIX;
> -		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
> +		if (is_abstract) {
> +			// abstract address

Please don't prefer c99 comment sytle, there are a few occurrences below.

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, October 5, 2020 3:09 PM
> To: Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)
> <jgrajcia@cisco.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address
> 
> On 10/5/2020 1:39 PM, Jakub Grajciar wrote:
> > Abstract socket address has no connection with filesystem pathnames
> > and the socket dissapears once all open references are closed.
> >
> > Memif pmd will use abstract socket address by default.
> > For backwards compatibility use new argument 'socket-abstract=no'
> >
> 
> Why this backward compatibility is required? How the end user affected from
> swithching to abstract sockets?
> Since when linux supports abstract sockets, does this switch will cause problem
> with old kernel versions?
> 
> Is there any benefit of the abstract sockets other than socket cleaned
> automatically (I assume for unix sockets it is done when file filesystem reference
> removed)?
> 

What I mean by this is compatibility with other implementations and older versions of the driver, e.g. vpp and libmemif don't support abstract socket yet.

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Stephen Hemminger]

On Mon, 5 Oct 2020 14:09:20 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 10/5/2020 1:39 PM, Jakub Grajciar wrote:
> > Abstract socket address has no connection with
> > filesystem pathnames and the socket dissapears
> > once all open references are closed.
> > 
> > Memif pmd will use abstract socket address by default.
> > For backwards compatibility use new argument
> > 'socket-abstract=no'
> >   
> 
> Why this backward compatibility is required? How the end user affected from 
> swithching to abstract sockets?

It would only matter if mixing applications with different versions.

> Since when linux supports abstract sockets, does this switch will cause problem 
> with old kernel versions?

This is not new, it dates back to Linux 2.4 or earlier.

> 
> Is there any benefit of the abstract sockets other than socket cleaned 
> automatically (I assume for unix sockets it is done when file filesystem 
> reference removed)?

The big one is that applications don't have to blindly unlink the old filesystem
remnant. This means that if application can't bind it means another application
is still running with that name. So abstract sockets are safer.


Abstract sockets are not pathnames so they get handled differently by security
systems (like SELinux and AppArmor). This can be helpful in containers.

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Ferruh Yigit]

On 10/5/2020 7:17 PM, Stephen Hemminger wrote:
> On Mon, 5 Oct 2020 14:09:20 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 10/5/2020 1:39 PM, Jakub Grajciar wrote:
>>> Abstract socket address has no connection with
>>> filesystem pathnames and the socket dissapears
>>> once all open references are closed.
>>>
>>> Memif pmd will use abstract socket address by default.
>>> For backwards compatibility use new argument
>>> 'socket-abstract=no'
>>>    
>>
>> Why this backward compatibility is required? How the end user affected from
>> swithching to abstract sockets?
> 
> It would only matter if mixing applications with different versions.
> 
>> Since when linux supports abstract sockets, does this switch will cause problem
>> with old kernel versions?
> 
> This is not new, it dates back to Linux 2.4 or earlier.
> 
>>
>> Is there any benefit of the abstract sockets other than socket cleaned
>> automatically (I assume for unix sockets it is done when file filesystem
>> reference removed)?
> 
> The big one is that applications don't have to blindly unlink the old filesystem
> remnant. This means that if application can't bind it means another application
> is still running with that name. So abstract sockets are safer.
> 
> 
> Abstract sockets are not pathnames so they get handled differently by security
> systems (like SELinux and AppArmor). This can be helpful in containers.
> 

Hi Stephen, thank you for clarification.

[dpdk-dev] [PATCH v2] net/memif: use abstract socket address

[Jakub Grajciar]

Abstract socket address has no connection with
filesystem pathnames and the socket dissapears
once all open references are closed.

Memif pmd will use abstract socket address by default.
For backwards compatibility use new argument
'socket-abstract=no'

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
---
 doc/guides/nics/memif.rst         |  1 +
 drivers/net/memif/memif_socket.c  | 28 +++++++++++++++++--------
 drivers/net/memif/rte_eth_memif.c | 34 ++++++++++++++++++++++++++++++-
 drivers/net/memif/rte_eth_memif.h | 10 +++++----
 4 files changed, 60 insertions(+), 13 deletions(-)

v2:
	- fixed coding style issues

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index ddeebed25..8e80105a4 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -43,6 +43,7 @@ client.
    "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
    "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
    "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
+   "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
    "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
    "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
    "zero-copy=yes", "Enable/disable zero-copy slave mode. Only relevant to slave, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 67794cb6f..8441f5397 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -862,10 +862,10 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 {
 	struct memif_socket *sock;
-	struct sockaddr_un un;
+	struct sockaddr_un un = { 0 };
 	int sockfd;
 	int ret;
 	int on = 1;
@@ -886,7 +886,13 @@ memif_socket_create(char *key, uint8_t listener)
 			goto error;
 
 		un.sun_family = AF_UNIX;
-		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		if (is_abstract) {
+			/* abstract address */
+			un.sun_path[0] = '\0';
+			strlcpy(un.sun_path + 1, sock->filename, MEMIF_SOCKET_UN_SIZE - 1);
+		} else {
+			strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		}
 
 		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
 				 sizeof(on));
@@ -963,7 +969,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	ret = rte_hash_lookup_data(hash, key, (void **)&socket);
 	if (ret < 0) {
 		socket = memif_socket_create(key,
-					     (pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1);
+			(pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1,
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
@@ -1025,7 +1032,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 	/* remove socket, if this was the last device using it */
 	if (TAILQ_EMPTY(&socket->dev_queue)) {
 		rte_hash_del_key(hash, socket->filename);
-		if (socket->listener) {
+		if (socket->listener && !(pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
 			/* remove listener socket file,
 			 * so we can create new one later.
 			 */
@@ -1054,7 +1061,7 @@ memif_connect_slave(struct rte_eth_dev *dev)
 {
 	int sockfd;
 	int ret;
-	struct sockaddr_un sun;
+	struct sockaddr_un sun = { 0 };
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	memset(pmd->local_disc_string, 0, ETH_MEMIF_DISC_STRING_SIZE);
@@ -1068,8 +1075,13 @@ memif_connect_slave(struct rte_eth_dev *dev)
 	}
 
 	sun.sun_family = AF_UNIX;
-
-	memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
+	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
+		/* abstract address */
+		sun.sun_path[0] = '\0';
+		memcpy(sun.sun_path + 1, pmd->socket_filename, sizeof(sun.sun_path) - 2);
+	} else {
+		memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
+	}
 
 	ret = connect(sockfd, (struct sockaddr *)&sun,
 		      sizeof(struct sockaddr_un));
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index c1c7e9f8d..55c865d44 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -36,6 +36,7 @@
 #define ETH_MEMIF_PKT_BUFFER_SIZE_ARG	"bsize"
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
+#define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -46,6 +47,7 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
+	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1640,7 +1642,23 @@ memif_set_socket_filename(const char *key __rte_unused, const char *value,
 	const char **socket_filename = (const char **)extra_args;
 
 	*socket_filename = value;
-	return memif_check_socket_filename(*socket_filename);
+	return 0;
+}
+
+static int
+memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	uint32_t *flags = (uint32_t *)extra_args;
+
+	if (strstr(value, "yes") != NULL) {
+		*flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else if (strstr(value, "no") != NULL) {
+		*flags &= ~ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else {
+		MIF_LOG(ERR, "Failed to parse socket-abstract param: %s.", value);
+		return -EINVAL;
+	}
+	return 0;
 }
 
 static int
@@ -1726,6 +1744,9 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 		MIF_LOG(WARNING, "Failed to register mp action callback: %s",
 			strerror(rte_errno));
 
+	/* use abstract address by default */
+	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
 
 	/* parse parameters */
@@ -1751,6 +1772,10 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 (void *)(&socket_filename));
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+					 &memif_set_is_socket_abstract, &flags);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1765,6 +1790,12 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 			goto exit;
 	}
 
+	if (!(flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
+		ret = memif_check_socket_filename(socket_filename);
+		if (ret < 0)
+			goto exit;
+	}
+
 	/* create interface */
 	ret = memif_create(vdev, role, id, flags, socket_filename,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
@@ -1802,6 +1833,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
+				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 6f45b7072..ac0a2f2d1 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -81,14 +81,16 @@ struct pmd_internals {
 	memif_interface_id_t id;		/**< unique id */
 	enum memif_role_t role;			/**< device role */
 	uint32_t flags;				/**< device status flags */
-#define ETH_MEMIF_FLAG_CONNECTING	(1 << 0)
+#define ETH_MEMIF_FLAG_CONNECTING		(1 << 0)
 /**< device is connecting */
-#define ETH_MEMIF_FLAG_CONNECTED	(1 << 1)
+#define ETH_MEMIF_FLAG_CONNECTED		(1 << 1)
 /**< device is connected */
-#define ETH_MEMIF_FLAG_ZERO_COPY	(1 << 2)
+#define ETH_MEMIF_FLAG_ZERO_COPY		(1 << 2)
 /**< device is zero-copy enabled */
-#define ETH_MEMIF_FLAG_DISABLED		(1 << 3)
+#define ETH_MEMIF_FLAG_DISABLED			(1 << 3)
 /**< device has not been configured and can not accept connection requests */
+#define ETH_MEMIF_FLAG_SOCKET_ABSTRACT	(1 << 4)
+/**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
-- 
2.17.1

[dpdk-dev] [PATCH v3] net/memif: use abstract socket address

[Jakub Grajciar]

Abstract socket address has no connection with
filesystem pathnames and the socket disappears
once all open references are closed.

Memif pmd will use abstract socket address by default.
For backwards compatibility use new argument
'socket-abstract=no'

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
---
 doc/guides/nics/memif.rst         |  1 +
 drivers/net/memif/memif_socket.c  | 28 +++++++++++++++++--------
 drivers/net/memif/rte_eth_memif.c | 34 ++++++++++++++++++++++++++++++-
 drivers/net/memif/rte_eth_memif.h | 10 +++++----
 4 files changed, 60 insertions(+), 13 deletions(-)

v2:
	- fixed coding style issues
v3:
	- use strlcpy for socket address

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index ddeebed25..8e80105a4 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -43,6 +43,7 @@ client.
    "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
    "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
    "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
+   "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
    "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
    "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
    "zero-copy=yes", "Enable/disable zero-copy slave mode. Only relevant to slave, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 67794cb6f..10f53ccb3 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -862,10 +862,10 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 {
 	struct memif_socket *sock;
-	struct sockaddr_un un;
+	struct sockaddr_un un = { 0 };
 	int sockfd;
 	int ret;
 	int on = 1;
@@ -886,7 +886,13 @@ memif_socket_create(char *key, uint8_t listener)
 			goto error;
 
 		un.sun_family = AF_UNIX;
-		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		if (is_abstract) {
+			/* abstract address */
+			un.sun_path[0] = '\0';
+			strlcpy(un.sun_path + 1, sock->filename, MEMIF_SOCKET_UN_SIZE - 1);
+		} else {
+			strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		}
 
 		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
 				 sizeof(on));
@@ -963,7 +969,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	ret = rte_hash_lookup_data(hash, key, (void **)&socket);
 	if (ret < 0) {
 		socket = memif_socket_create(key,
-					     (pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1);
+			(pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1,
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
@@ -1025,7 +1032,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 	/* remove socket, if this was the last device using it */
 	if (TAILQ_EMPTY(&socket->dev_queue)) {
 		rte_hash_del_key(hash, socket->filename);
-		if (socket->listener) {
+		if (socket->listener && !(pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
 			/* remove listener socket file,
 			 * so we can create new one later.
 			 */
@@ -1054,7 +1061,7 @@ memif_connect_slave(struct rte_eth_dev *dev)
 {
 	int sockfd;
 	int ret;
-	struct sockaddr_un sun;
+	struct sockaddr_un sun = { 0 };
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	memset(pmd->local_disc_string, 0, ETH_MEMIF_DISC_STRING_SIZE);
@@ -1068,8 +1075,13 @@ memif_connect_slave(struct rte_eth_dev *dev)
 	}
 
 	sun.sun_family = AF_UNIX;
-
-	memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
+	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
+		/* abstract address */
+		sun.sun_path[0] = '\0';
+		strlcpy(sun.sun_path + 1,  pmd->socket_filename, MEMIF_SOCKET_UN_SIZE - 1);
+	} else {
+		strlcpy(sun.sun_path,  pmd->socket_filename, MEMIF_SOCKET_UN_SIZE);
+	}
 
 	ret = connect(sockfd, (struct sockaddr *)&sun,
 		      sizeof(struct sockaddr_un));
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index c1c7e9f8d..55c865d44 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -36,6 +36,7 @@
 #define ETH_MEMIF_PKT_BUFFER_SIZE_ARG	"bsize"
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
+#define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -46,6 +47,7 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
+	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1640,7 +1642,23 @@ memif_set_socket_filename(const char *key __rte_unused, const char *value,
 	const char **socket_filename = (const char **)extra_args;
 
 	*socket_filename = value;
-	return memif_check_socket_filename(*socket_filename);
+	return 0;
+}
+
+static int
+memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	uint32_t *flags = (uint32_t *)extra_args;
+
+	if (strstr(value, "yes") != NULL) {
+		*flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else if (strstr(value, "no") != NULL) {
+		*flags &= ~ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else {
+		MIF_LOG(ERR, "Failed to parse socket-abstract param: %s.", value);
+		return -EINVAL;
+	}
+	return 0;
 }
 
 static int
@@ -1726,6 +1744,9 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 		MIF_LOG(WARNING, "Failed to register mp action callback: %s",
 			strerror(rte_errno));
 
+	/* use abstract address by default */
+	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
 
 	/* parse parameters */
@@ -1751,6 +1772,10 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 (void *)(&socket_filename));
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+					 &memif_set_is_socket_abstract, &flags);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1765,6 +1790,12 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 			goto exit;
 	}
 
+	if (!(flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
+		ret = memif_check_socket_filename(socket_filename);
+		if (ret < 0)
+			goto exit;
+	}
+
 	/* create interface */
 	ret = memif_create(vdev, role, id, flags, socket_filename,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
@@ -1802,6 +1833,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
+				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 6f45b7072..ac0a2f2d1 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -81,14 +81,16 @@ struct pmd_internals {
 	memif_interface_id_t id;		/**< unique id */
 	enum memif_role_t role;			/**< device role */
 	uint32_t flags;				/**< device status flags */
-#define ETH_MEMIF_FLAG_CONNECTING	(1 << 0)
+#define ETH_MEMIF_FLAG_CONNECTING		(1 << 0)
 /**< device is connecting */
-#define ETH_MEMIF_FLAG_CONNECTED	(1 << 1)
+#define ETH_MEMIF_FLAG_CONNECTED		(1 << 1)
 /**< device is connected */
-#define ETH_MEMIF_FLAG_ZERO_COPY	(1 << 2)
+#define ETH_MEMIF_FLAG_ZERO_COPY		(1 << 2)
 /**< device is zero-copy enabled */
-#define ETH_MEMIF_FLAG_DISABLED		(1 << 3)
+#define ETH_MEMIF_FLAG_DISABLED			(1 << 3)
 /**< device has not been configured and can not accept connection requests */
+#define ETH_MEMIF_FLAG_SOCKET_ABSTRACT	(1 << 4)
+/**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3] net/memif: use abstract socket address

[Ferruh Yigit]

On 10/9/2020 5:58 PM, Jakub Grajciar wrote:
> Abstract socket address has no connection with
> filesystem pathnames and the socket disappears
> once all open references are closed.
> 
> Memif pmd will use abstract socket address by default.
> For backwards compatibility use new argument
> 'socket-abstract=no'
> 
> Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
> ---
>   doc/guides/nics/memif.rst         |  1 +
>   drivers/net/memif/memif_socket.c  | 28 +++++++++++++++++--------
>   drivers/net/memif/rte_eth_memif.c | 34 ++++++++++++++++++++++++++++++-
>   drivers/net/memif/rte_eth_memif.h | 10 +++++----
>   4 files changed, 60 insertions(+), 13 deletions(-)

looks good thanks, can you also add a release notes update for the feature?

[dpdk-dev] [PATCH v4] net/memif: use abstract socket address

[Jakub Grajciar]

Abstract socket address has no connection with
filesystem pathnames and the socket disappears
once all open references are closed.

Memif pmd will use abstract socket address by default.
For backwards compatibility use new argument
'socket-abstract=no'

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
---
 doc/guides/nics/memif.rst              |  1 +
 doc/guides/rel_notes/release_20_11.rst |  5 ++++
 drivers/net/memif/memif_socket.c       | 28 +++++++++++++++------
 drivers/net/memif/rte_eth_memif.c      | 34 +++++++++++++++++++++++++-
 drivers/net/memif/rte_eth_memif.h      | 10 +++++---
 5 files changed, 65 insertions(+), 13 deletions(-)

v2:
	- fixed coding style issues
v3:
	- use strlcpy for socket address
v4:
	- updated release notes

diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst
index ddeebed25..8e80105a4 100644
--- a/doc/guides/nics/memif.rst
+++ b/doc/guides/nics/memif.rst
@@ -43,6 +43,7 @@ client.
    "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"
    "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"
    "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"
+   "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"
    "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""
    "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"
    "zero-copy=yes", "Enable/disable zero-copy slave mode. Only relevant to slave, requires '--single-file-segments' eal argument", "no", "yes|no"
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 57e3edcdd..62891bed0 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -92,6 +92,11 @@ New Features
   * Changed default link speed to unknown.
   * Added support for 200G link speed.
 
+* **Updated Memif PMD.**
+
+  * Added support for abstract socket address.
+  * Changed default socket address type to abstract.
+
 * **Added Ice Lake (Gen4) support for Intel NTB.**
 
   Added NTB device support (4th generation) for Intel Ice Lake platform.
diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 67794cb6f..7a26e3006 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -862,10 +862,10 @@ memif_listener_handler(void *arg)
 }
 
 static struct memif_socket *
-memif_socket_create(char *key, uint8_t listener)
+memif_socket_create(char *key, uint8_t listener, bool is_abstract)
 {
 	struct memif_socket *sock;
-	struct sockaddr_un un;
+	struct sockaddr_un un = { 0 };
 	int sockfd;
 	int ret;
 	int on = 1;
@@ -886,7 +886,13 @@ memif_socket_create(char *key, uint8_t listener)
 			goto error;
 
 		un.sun_family = AF_UNIX;
-		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		if (is_abstract) {
+			/* abstract address */
+			un.sun_path[0] = '\0';
+			strlcpy(un.sun_path + 1, sock->filename, MEMIF_SOCKET_UN_SIZE - 1);
+		} else {
+			strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
+		}
 
 		ret = setsockopt(sockfd, SOL_SOCKET, SO_PASSCRED, &on,
 				 sizeof(on));
@@ -963,7 +969,8 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 	ret = rte_hash_lookup_data(hash, key, (void **)&socket);
 	if (ret < 0) {
 		socket = memif_socket_create(key,
-					     (pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1);
+			(pmd->role == MEMIF_ROLE_SLAVE) ? 0 : 1,
+			pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT);
 		if (socket == NULL)
 			return -1;
 		ret = rte_hash_add_key_data(hash, key, socket);
@@ -1025,7 +1032,7 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 	/* remove socket, if this was the last device using it */
 	if (TAILQ_EMPTY(&socket->dev_queue)) {
 		rte_hash_del_key(hash, socket->filename);
-		if (socket->listener) {
+		if (socket->listener && !(pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
 			/* remove listener socket file,
 			 * so we can create new one later.
 			 */
@@ -1054,7 +1061,7 @@ memif_connect_slave(struct rte_eth_dev *dev)
 {
 	int sockfd;
 	int ret;
-	struct sockaddr_un sun;
+	struct sockaddr_un sun = { 0 };
 	struct pmd_internals *pmd = dev->data->dev_private;
 
 	memset(pmd->local_disc_string, 0, ETH_MEMIF_DISC_STRING_SIZE);
@@ -1068,8 +1075,13 @@ memif_connect_slave(struct rte_eth_dev *dev)
 	}
 
 	sun.sun_family = AF_UNIX;
-
-	memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
+	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
+		/* abstract address */
+		sun.sun_path[0] = '\0';
+		strlcpy(sun.sun_path + 1,  pmd->socket_filename, MEMIF_SOCKET_UN_SIZE - 1);
+	} else {
+		strlcpy(sun.sun_path,  pmd->socket_filename, MEMIF_SOCKET_UN_SIZE);
+	}
 
 	ret = connect(sockfd, (struct sockaddr *)&sun,
 		      sizeof(struct sockaddr_un));
diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index ff8a58081..6f4596667 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -36,6 +36,7 @@
 #define ETH_MEMIF_PKT_BUFFER_SIZE_ARG	"bsize"
 #define ETH_MEMIF_RING_SIZE_ARG		"rsize"
 #define ETH_MEMIF_SOCKET_ARG		"socket"
+#define ETH_MEMIF_SOCKET_ABSTRACT_ARG	"socket-abstract"
 #define ETH_MEMIF_MAC_ARG		"mac"
 #define ETH_MEMIF_ZC_ARG		"zero-copy"
 #define ETH_MEMIF_SECRET_ARG		"secret"
@@ -46,6 +47,7 @@ static const char * const valid_arguments[] = {
 	ETH_MEMIF_PKT_BUFFER_SIZE_ARG,
 	ETH_MEMIF_RING_SIZE_ARG,
 	ETH_MEMIF_SOCKET_ARG,
+	ETH_MEMIF_SOCKET_ABSTRACT_ARG,
 	ETH_MEMIF_MAC_ARG,
 	ETH_MEMIF_ZC_ARG,
 	ETH_MEMIF_SECRET_ARG,
@@ -1654,7 +1656,23 @@ memif_set_socket_filename(const char *key __rte_unused, const char *value,
 	const char **socket_filename = (const char **)extra_args;
 
 	*socket_filename = value;
-	return memif_check_socket_filename(*socket_filename);
+	return 0;
+}
+
+static int
+memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	uint32_t *flags = (uint32_t *)extra_args;
+
+	if (strstr(value, "yes") != NULL) {
+		*flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else if (strstr(value, "no") != NULL) {
+		*flags &= ~ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+	} else {
+		MIF_LOG(ERR, "Failed to parse socket-abstract param: %s.", value);
+		return -EINVAL;
+	}
+	return 0;
 }
 
 static int
@@ -1740,6 +1758,9 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 		MIF_LOG(WARNING, "Failed to register mp action callback: %s",
 			strerror(rte_errno));
 
+	/* use abstract address by default */
+	flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
+
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);
 
 	/* parse parameters */
@@ -1765,6 +1786,10 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 					 (void *)(&socket_filename));
 		if (ret < 0)
 			goto exit;
+		ret = rte_kvargs_process(kvlist, ETH_MEMIF_SOCKET_ABSTRACT_ARG,
+					 &memif_set_is_socket_abstract, &flags);
+		if (ret < 0)
+			goto exit;
 		ret = rte_kvargs_process(kvlist, ETH_MEMIF_MAC_ARG,
 					 &memif_set_mac, ether_addr);
 		if (ret < 0)
@@ -1779,6 +1804,12 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)
 			goto exit;
 	}
 
+	if (!(flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT)) {
+		ret = memif_check_socket_filename(socket_filename);
+		if (ret < 0)
+			goto exit;
+	}
+
 	/* create interface */
 	ret = memif_create(vdev, role, id, flags, socket_filename,
 			   log2_ring_size, pkt_buffer_size, secret, ether_addr);
@@ -1816,6 +1847,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_memif,
 			      ETH_MEMIF_PKT_BUFFER_SIZE_ARG "=<int>"
 			      ETH_MEMIF_RING_SIZE_ARG "=<int>"
 			      ETH_MEMIF_SOCKET_ARG "=<string>"
+				  ETH_MEMIF_SOCKET_ABSTRACT_ARG "=yes|no"
 			      ETH_MEMIF_MAC_ARG "=xx:xx:xx:xx:xx:xx"
 			      ETH_MEMIF_ZC_ARG "=yes|no"
 			      ETH_MEMIF_SECRET_ARG "=<string>");
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index 6f45b7072..ac0a2f2d1 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -81,14 +81,16 @@ struct pmd_internals {
 	memif_interface_id_t id;		/**< unique id */
 	enum memif_role_t role;			/**< device role */
 	uint32_t flags;				/**< device status flags */
-#define ETH_MEMIF_FLAG_CONNECTING	(1 << 0)
+#define ETH_MEMIF_FLAG_CONNECTING		(1 << 0)
 /**< device is connecting */
-#define ETH_MEMIF_FLAG_CONNECTED	(1 << 1)
+#define ETH_MEMIF_FLAG_CONNECTED		(1 << 1)
 /**< device is connected */
-#define ETH_MEMIF_FLAG_ZERO_COPY	(1 << 2)
+#define ETH_MEMIF_FLAG_ZERO_COPY		(1 << 2)
 /**< device is zero-copy enabled */
-#define ETH_MEMIF_FLAG_DISABLED		(1 << 3)
+#define ETH_MEMIF_FLAG_DISABLED			(1 << 3)
 /**< device has not been configured and can not accept connection requests */
+#define ETH_MEMIF_FLAG_SOCKET_ABSTRACT	(1 << 4)
+/**< use abstract socket address */
 
 	char *socket_filename;			/**< pointer to socket filename */
 	char secret[ETH_MEMIF_SECRET_SIZE]; /**< secret (optional security parameter) */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4] net/memif: use abstract socket address

[Ferruh Yigit]

On 10/12/2020 9:28 AM, Jakub Grajciar wrote:
> Abstract socket address has no connection with
> filesystem pathnames and the socket disappears
> once all open references are closed.
> 
> Memif pmd will use abstract socket address by default.
> For backwards compatibility use new argument
> 'socket-abstract=no'
> 
> Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
>

Applied to dpdk-next-net/main, thanks.

Re: [dpdk-dev] [PATCH v4] net/memif: use abstract socket address

[Stephen Hemminger]

On Mon, 12 Oct 2020 10:28:29 +0200
Jakub Grajciar <jgrajcia@cisco.com> wrote:

> Abstract socket address has no connection with
> filesystem pathnames and the socket disappears
> once all open references are closed.
> 
> Memif pmd will use abstract socket address by default.
> For backwards compatibility use new argument
> 'socket-abstract=no'
> 
> Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>

Your current model with flags is fine, but another alternative would
be to use the @ character to signify abstract socket type.
That is what tools like "ss" use display abstract sockets.

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Stephen Hemminger]

On Mon, 5 Oct 2020 14:39:00 +0200
Jakub Grajciar <jgrajcia@cisco.com> wrote:

> @@ -886,7 +886,12 @@ memif_socket_create(char *key, uint8_t listener)
>  			goto error;
>  
>  		un.sun_family = AF_UNIX;
> -		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
> +		if (is_abstract) {
> +			// abstract address
No C++ comments please.

> +			un.sun_path[0] = '\0';
Already set to zero when initialized.

> +	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
> +		// abstract address
ditto no C++ comments

> +		sun.sun_path[0] = '\0';
again zeroed again

> +		memcpy(sun.sun_path + 1, pmd->socket_filename, sizeof(sun.sun_path) - 2);
> +	} else
> +		memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);

This code is buggy since it potentially reads past the end of the strinc in filename.
Use strlcpy here instead.

Re: [dpdk-dev] [PATCH] net/memif: use abstract socket address

[Ferruh Yigit]

On 10/7/2020 4:03 PM, Stephen Hemminger wrote:
> On Mon, 5 Oct 2020 14:39:00 +0200
> Jakub Grajciar <jgrajcia@cisco.com> wrote:
> 
>> @@ -886,7 +886,12 @@ memif_socket_create(char *key, uint8_t listener)
>>   			goto error;
>>   
>>   		un.sun_family = AF_UNIX;
>> -		strlcpy(un.sun_path, sock->filename, MEMIF_SOCKET_UN_SIZE);
>> +		if (is_abstract) {
>> +			// abstract address
> No C++ comments please.
> 
>> +			un.sun_path[0] = '\0';
> Already set to zero when initialized.
> 
>> +	if (pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT) {
>> +		// abstract address
> ditto no C++ comments
> 
>> +		sun.sun_path[0] = '\0';
> again zeroed again
> 
>> +		memcpy(sun.sun_path + 1, pmd->socket_filename, sizeof(sun.sun_path) - 2);
>> +	} else
>> +		memcpy(sun.sun_path, pmd->socket_filename, sizeof(sun.sun_path) - 1);
> 
> This code is buggy since it potentially reads past the end of the strinc in filename.
> Use strlcpy here instead.
> 

Hi Jakub,

v2 is still using memcpy, have you seen Stephen's comment above?