On 12/7/2022 3:56 PM, Nathan Skrzypczak wrote: > Hi Ferruh, > Hi Nathan,
> Thank you for your reply, > > On the potential confusion for users of the DPDK memif PMD : when > defaulting to abstract sockets was added in [0] (v20.10 release) > it did change the existing behavior, so reverting it would restore the > old behavior.> 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 > regular sockets because they're way easier to handle. > Not sure if regular socket is easier to handle, or users prefer regular sockets, we need more input on these. > 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. > What do you think ? > I don't have a preference about the default config, but I also don't have enough justification for changing the current behavior. @Jakub, as driver maintainer, do you have any preference? > Cheers > -Nathan > > [0] 2f865ed07b > > Le mer. 7 déc. 2022 à 16:14, Ferruh Yigit <ferruh.yi...@amd.com > <mailto:ferruh.yi...@amd.com>> a écrit : > > On 10/17/2022 1:12 PM, Nathan Skrzypczak wrote: > > This patch changes the default value of the memif abstract > > socket flag to false. The implementation of memif with > > abstract sockets made abstract-socket=yes the > > default in [0] which might confuse users. > > > > Hi Nathan, > > OK to update logs to clarify nature of the socket, > > but why do you think making abstract socket default socket type confuses > the users? > > > This patches makes 'socket-abstract=no' the new default, > > and adds warnings mentioning the nature of the socket > > concerned in an attempt to avoid future foot-gunning. > > > > [0] commit 2f865ed07bb6 ("net/memif: use abstract socket address") > > > > Signed-off-by: Nathan Skrzypczak <nathan.skrzypc...@gmail.com > <mailto:nathan.skrzypc...@gmail.com>> > > --- > > doc/guides/nics/memif.rst | 2 +- > > drivers/net/memif/memif_socket.c | 7 +++++-- > > drivers/net/memif/rte_eth_memif.c | 3 --- > > 3 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/doc/guides/nics/memif.rst b/doc/guides/nics/memif.rst > > index aca843640b..43d1cd1b38 100644 > > --- a/doc/guides/nics/memif.rst > > +++ b/doc/guides/nics/memif.rst > > @@ -43,7 +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" > > + "socket-abstract=no", "Is the socket an abstract socket", > "no", "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 client mode. Only > relevant to client, 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 4700ce2e77..5344e60147 100644 > > --- a/drivers/net/memif/memif_socket.c > > +++ b/drivers/net/memif/memif_socket.c > > @@ -939,7 +939,8 @@ memif_socket_create(char *key, uint8_t > listener, bool is_abstract) > > if (ret < 0) > > goto error; > > > > - MIF_LOG(DEBUG, "Memif listener socket %s created.", > sock->filename); > > + MIF_LOG(DEBUG, "Memif listener %s socket %s created.", > > + is_abstract ? "abstract" : "", sock->filename); > > > > /* Allocate interrupt instance */ > > sock->intr_handle = > > @@ -1139,7 +1140,9 @@ memif_connect_client(struct rte_eth_dev *dev) > > > > ret = connect(sockfd, (struct sockaddr *)&sun, sunlen); > > if (ret < 0) { > > - MIF_LOG(ERR, "Failed to connect socket: %s.", > pmd->socket_filename); > > + MIF_LOG(ERR, "Failed to connect %s socket: %s.", > > + pmd->flags & ETH_MEMIF_FLAG_SOCKET_ABSTRACT ? > "abstract" : "", > > + pmd->socket_filename); > > goto error; > > } > > > > diff --git a/drivers/net/memif/rte_eth_memif.c > b/drivers/net/memif/rte_eth_memif.c > > index 5b5cae34ea..81e502ccaf 100644 > > --- a/drivers/net/memif/rte_eth_memif.c > > +++ b/drivers/net/memif/rte_eth_memif.c > > @@ -1823,9 +1823,6 @@ 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 */ >
