<div dir="ltr"><div>Hi Ferruh,</div><div><br></div><div>Thank you for your reply, </div><div><br></div><div>On the potential confusion for users of the DPDK memif PMD : when defaulting to abstract sockets was added in [0] (v20.10 release) <br></div><div>it did change the existing behavior, so reverting it would restore the old behavior. <br></div><div>Also abstract sockets are quite a unusual feature in linux (a 0byte prefixed string...), so I'm expecting most users of memif to just use</div><div>regular sockets because they're way easier to handle.</div><div><br></div><div>Also my guess is it will probably bug people if the way to use regular sockets is to pass `abstract-socket=false` to the PMD config.<br></div><div>What do you think ?</div><div><br></div><div>Cheers</div><div>-Nathan<br></div><div><br></div><div>[0] 2f865ed07b</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le mer. 7 déc. 2022 à 16:14, Ferruh Yigit <<a href="mailto:ferruh.yigit@amd.com">ferruh.yigit@amd.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 10/17/2022 1:12 PM, Nathan Skrzypczak wrote:<br>
> This patch changes the default value of the memif abstract<br>
> socket flag to false. The implementation of memif with<br>
> abstract sockets made abstract-socket=yes the<br>
> default in [0] which might confuse users.<br>
> <br>
<br>
Hi Nathan,<br>
<br>
OK to update logs to clarify nature of the socket,<br>
<br>
but why do you think making abstract socket default socket type confuses<br>
the users?<br>
<br>
> This patches makes 'socket-abstract=no' the new default,<br>
> and adds warnings mentioning the nature of the socket<br>
> concerned in an attempt to avoid future foot-gunning.<br>
> <br>
> [0] commit 2f865ed07bb6 ("net/memif: use abstract socket address")<br>
> <br>
> Signed-off-by: Nathan Skrzypczak <<a href="mailto:nathan.skrzypczak@gmail.com" target="_blank">nathan.skrzypczak@gmail.com</a>><br>
> ---<br>
> doc/guides/nics/memif.rst | 2 +-<br>
> drivers/net/memif/memif_socket.c | 7 +++++--<br>
> drivers/net/memif/rte_eth_memif.c | 3 ---<br>
> 3 files changed, 6 insertions(+), 6 deletions(-)<br>
> <br>
> diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst<br>
> index aca843640b..43d1cd1b38 100644<br>
> --- a/doc/guides/nics/memif.rst<br>
> +++ b/doc/guides/nics/memif.rst<br>
> @@ -43,7 +43,7 @@ client.<br>
> "bsize=1024", "Size of single packet buffer", "2048", "uint16_t"<br>
> "rsize=11", "Log2 of ring size. If rsize is 10, actual ring size is 1024", "10", "1-14"<br>
> "socket=/tmp/memif.sock", "Socket filename", "/tmp/memif.sock", "string len 108"<br>
> - "socket-abstract=no", "Set usage of abstract socket address", "yes", "yes|no"<br>
> + "socket-abstract=no", "Is the socket an abstract socket", "no", "yes|no"<br>
> "mac=01:23:45:ab:cd:ef", "Mac address", "01:ab:23:cd:45:ef", ""<br>
> "secret=abc123", "Secret is an optional security option, which if specified, must be matched by peer", "", "string len 24"<br>
> "zero-copy=yes", "Enable/disable zero-copy client mode. Only relevant to client, requires '--single-file-segments' eal argument", "no", "yes|no"<br>
> diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c<br>
> index 4700ce2e77..5344e60147 100644<br>
> --- a/drivers/net/memif/memif_socket.c<br>
> +++ b/drivers/net/memif/memif_socket.c<br>
> @@ -939,7 +939,8 @@ memif_socket_create(char *key, uint8_t listener, bool is_abstract)<br>
> if (ret < 0)<br>
> goto error;<br>
> <br>
> - MIF_LOG(DEBUG, "Memif listener socket %s created.", sock->filename);<br>
> + MIF_LOG(DEBUG, "Memif listener %s socket %s created.",<br>
> + is_abstract ? "abstract" : "", sock->filename);<br>
> <br>
> /* Allocate interrupt instance */<br>
> sock->intr_handle =<br>
> @@ -1139,7 +1140,9 @@ memif_connect_client(struct rte_eth_dev *dev)<br>
> <br>
> ret = connect(sockfd, (struct sockaddr *)&sun, sunlen);<br>
> if (ret < 0) {<br>
> - MIF_LOG(ERR, "Failed to connect socket: %s.", pmd->socket_filename);<br>
> + MIF_LOG(ERR, "Failed to connect %s socket: %s.",<br>
> + pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT ? "abstract" : "",<br>
> + pmd->socket_filename);<br>
> goto error;<br>
> }<br>
> <br>
> diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c<br>
> index 5b5cae34ea..81e502ccaf 100644<br>
> --- a/drivers/net/memif/rte_eth_memif.c<br>
> +++ b/drivers/net/memif/rte_eth_memif.c<br>
> @@ -1823,9 +1823,6 @@ rte_pmd_memif_probe(struct rte_vdev_device *vdev)<br>
> MIF_LOG(WARNING, "Failed to register mp action callback: %s",<br>
> strerror(rte_errno));<br>
> <br>
> - /* use abstract address by default */<br>
> - flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;<br>
> -<br>
> kvlist = rte_kvargs_parse(rte_vdev_device_args(vdev), valid_arguments);<br>
> <br>
> /* parse parameters */<br>
<br>
</blockquote></div>