DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/4] net/tap: minor cleanups
@ 2024-10-28 16:32 Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 1/4] net/tap: replace rte_memcpy Stephen Hemminger
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Stephen Hemminger @ 2024-10-28 16:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

These are some minor things in tap device that can be easily fixed.
Not urgent can wait until 25.03 release but submitting now for review.

Stephen Hemminger (4):
  net/tap: replace rte_memcpy
  net/tap: rename struct nlmsg to tap_nlmsg
  net/tap: do not use opaque parameter in convert
  net/tap: do not use rte_malloc for process private data

 drivers/net/tap/rte_eth_tap.c | 37 +++++++++----------
 drivers/net/tap/tap_flow.c    | 69 ++++++++++++++---------------------
 drivers/net/tap/tap_netlink.c |  4 +-
 drivers/net/tap/tap_netlink.h |  6 +--
 drivers/net/tap/tap_tcmsgs.c  | 10 ++---
 drivers/net/tap/tap_tcmsgs.h  |  2 +-
 6 files changed, 56 insertions(+), 72 deletions(-)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] net/tap: replace rte_memcpy
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
@ 2024-10-28 16:32 ` Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 2/4] net/tap: rename struct nlmsg to tap_nlmsg Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2024-10-28 16:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

There is no point in using rte_memcpy to copy Ethernet addresses.
Use rte_ether_addr_copy() instead.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 36b06b3ac5..da45610665 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -15,7 +15,6 @@
 #include <rte_net.h>
 #include <rte_debug.h>
 #include <rte_ip.h>
-#include <rte_string_fns.h>
 #include <rte_ethdev.h>
 #include <rte_errno.h>
 #include <rte_cycles.h>
@@ -1395,11 +1394,13 @@ tap_mac_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 			mac_addr))
 		mode = LOCAL_AND_REMOTE;
 	ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, RTE_ETHER_ADDR_LEN);
+
+	rte_ether_addr_copy(mac_addr, (struct rte_ether_addr *)&ifr.ifr_hwaddr.sa_data);
 	ret = tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode);
 	if (ret < 0)
 		return ret;
-	rte_memcpy(&pmd->eth_addr, mac_addr, RTE_ETHER_ADDR_LEN);
+
+	rte_ether_addr_copy(mac_addr, &pmd->eth_addr);
 
 #ifdef HAVE_TCA_FLOWER
 	if (pmd->remote_if_index && !pmd->flow_isolate) {
@@ -1987,7 +1988,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 		if (rte_is_zero_ether_addr(mac_addr))
 			rte_eth_random_addr((uint8_t *)&pmd->eth_addr);
 		else
-			rte_memcpy(&pmd->eth_addr, mac_addr, sizeof(*mac_addr));
+			rte_ether_addr_copy(mac_addr, &pmd->eth_addr);
 	}
 
 	/*
@@ -2010,8 +2011,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 	if (pmd->type == ETH_TUNTAP_TYPE_TAP) {
 		memset(&ifr, 0, sizeof(struct ifreq));
 		ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-		rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
-				RTE_ETHER_ADDR_LEN);
+		rte_ether_addr_copy(&pmd->eth_addr, (struct rte_ether_addr *)&ifr.ifr_hwaddr.sa_data);
 		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
 			goto error_exit;
 	}
@@ -2070,8 +2070,8 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 				pmd->name, pmd->remote_iface);
 			goto error_remote;
 		}
-		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data,
-			   RTE_ETHER_ADDR_LEN);
+
+		rte_ether_addr_copy((struct rte_ether_addr *)&ifr.ifr_hwaddr.sa_data, &pmd->eth_addr);
 		/* The desired MAC is already in ifreq after SIOCGIFHWADDR. */
 		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0) {
 			TAP_LOG(ERR, "%s: failed to get %s MAC address.",
-- 
2.45.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4] net/tap: rename struct nlmsg to tap_nlmsg
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 1/4] net/tap: replace rte_memcpy Stephen Hemminger
@ 2024-10-28 16:32 ` Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 3/4] net/tap: do not use opaque parameter in convert Stephen Hemminger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2024-10-28 16:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The struct netlink message structure is part of the internal
TAP device API, not the netlink API itself. Use tap_ prefix
to avoid any confusion with nlmsghdr from netlink.h

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/tap_flow.c    | 22 +++++++++++-----------
 drivers/net/tap/tap_netlink.c |  4 ++--
 drivers/net/tap/tap_netlink.h |  6 +++---
 drivers/net/tap/tap_tcmsgs.c  | 10 +++++-----
 drivers/net/tap/tap_tcmsgs.h  |  2 +-
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 51ec07eb5a..6564583174 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -35,7 +35,7 @@
 struct rte_flow {
 	LIST_ENTRY(rte_flow) next; /* Pointer to the next rte_flow structure */
 	struct rte_flow *remote_flow; /* associated remote flow */
-	struct nlmsg msg;
+	struct tap_nlmsg msg;
 };
 
 struct convert_data {
@@ -423,7 +423,7 @@ tap_flow_create_eth(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_eth *spec = item->spec;
 	const struct rte_flow_item_eth *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -477,7 +477,7 @@ tap_flow_create_vlan(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_vlan *spec = item->spec;
 	const struct rte_flow_item_vlan *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -537,7 +537,7 @@ tap_flow_create_ipv4(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_ipv4 *spec = item->spec;
 	const struct rte_flow_item_ipv4 *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -593,7 +593,7 @@ tap_flow_create_ipv6(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_ipv6 *mask = item->mask;
 	struct rte_flow *flow = info->flow;
 	uint8_t empty_addr[16] = { 0 };
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -648,7 +648,7 @@ tap_flow_create_udp(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_udp *spec = item->spec;
 	const struct rte_flow_item_udp *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -694,7 +694,7 @@ tap_flow_create_tcp(const struct rte_flow_item *item, void *data)
 	const struct rte_flow_item_tcp *spec = item->spec;
 	const struct rte_flow_item_tcp *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-	struct nlmsg *msg;
+	struct tap_nlmsg *msg;
 
 	/* use default mask if none provided */
 	if (!mask)
@@ -820,7 +820,7 @@ tap_flow_item_validate(const struct rte_flow_item *item,
 static int
 add_action(struct rte_flow *flow, size_t *act_index, struct action_data *adata)
 {
-	struct nlmsg *msg = &flow->msg;
+	struct tap_nlmsg *msg = &flow->msg;
 
 	if (tap_nlattr_nested_start(msg, (*act_index)++) < 0)
 		return -1;
@@ -891,7 +891,7 @@ static int
 add_actions(struct rte_flow *flow, int nb_actions, struct action_data *data,
 	    int classifier_action)
 {
-	struct nlmsg *msg = &flow->msg;
+	struct tap_nlmsg *msg = &flow->msg;
 	size_t act_index = 1;
 	int i;
 
@@ -1274,7 +1274,7 @@ tap_flow_create(struct rte_eth_dev *dev,
 	struct pmd_internals *pmd = dev->data->dev_private;
 	struct rte_flow *remote_flow = NULL;
 	struct rte_flow *flow = NULL;
-	struct nlmsg *msg = NULL;
+	struct tap_nlmsg *msg = NULL;
 	int err;
 
 	if (!pmd->if_index) {
@@ -1593,7 +1593,7 @@ int tap_flow_implicit_create(struct pmd_internals *pmd,
 	struct rte_flow_item_eth eth_local = { .hdr.ether_type = 0 };
 	unsigned int if_index = pmd->remote_if_index;
 	struct rte_flow *remote_flow = NULL;
-	struct nlmsg *msg = NULL;
+	struct tap_nlmsg *msg = NULL;
 	int err = 0;
 	struct rte_flow_item items_local[2] = {
 		[0] = {
diff --git a/drivers/net/tap/tap_netlink.c b/drivers/net/tap/tap_netlink.c
index 35c491ac37..8a57c9242c 100644
--- a/drivers/net/tap/tap_netlink.c
+++ b/drivers/net/tap/tap_netlink.c
@@ -368,7 +368,7 @@ tap_nlattr_add32(struct nlmsghdr *nh, unsigned short type, uint32_t data)
  *   -1 if adding a nested netlink attribute failed, 0 otherwise.
  */
 int
-tap_nlattr_nested_start(struct nlmsg *msg, uint16_t type)
+tap_nlattr_nested_start(struct tap_nlmsg *msg, uint16_t type)
 {
 	struct nested_tail *tail;
 
@@ -400,7 +400,7 @@ tap_nlattr_nested_start(struct nlmsg *msg, uint16_t type)
  *   The netlink message where to edit the nested_tails metadata.
  */
 void
-tap_nlattr_nested_finish(struct nlmsg *msg)
+tap_nlattr_nested_finish(struct tap_nlmsg *msg)
 {
 	struct nested_tail *tail = msg->nested_tails;
 
diff --git a/drivers/net/tap/tap_netlink.h b/drivers/net/tap/tap_netlink.h
index faa73ba163..466c47a6d7 100644
--- a/drivers/net/tap/tap_netlink.h
+++ b/drivers/net/tap/tap_netlink.h
@@ -16,7 +16,7 @@
 
 #define NLMSG_BUF 512
 
-struct nlmsg {
+struct tap_nlmsg {
 	struct nlmsghdr nh;
 	struct tcmsg t;
 	char buf[NLMSG_BUF];
@@ -36,7 +36,7 @@ void tap_nlattr_add(struct nlmsghdr *nh, unsigned short type,
 void tap_nlattr_add8(struct nlmsghdr *nh, unsigned short type, uint8_t data);
 void tap_nlattr_add16(struct nlmsghdr *nh, unsigned short type, uint16_t data);
 void tap_nlattr_add32(struct nlmsghdr *nh, unsigned short type, uint32_t data);
-int tap_nlattr_nested_start(struct nlmsg *msg, uint16_t type);
-void tap_nlattr_nested_finish(struct nlmsg *msg);
+int tap_nlattr_nested_start(struct tap_nlmsg *msg, uint16_t type);
+void tap_nlattr_nested_finish(struct tap_nlmsg *msg);
 
 #endif /* _TAP_NETLINK_H_ */
diff --git a/drivers/net/tap/tap_tcmsgs.c b/drivers/net/tap/tap_tcmsgs.c
index a3aae3c814..1755b57519 100644
--- a/drivers/net/tap/tap_tcmsgs.c
+++ b/drivers/net/tap/tap_tcmsgs.c
@@ -42,7 +42,7 @@ struct qdisc_custom_arg {
  *   Overrides the default netlink flags for this msg with those specified.
  */
 void
-tc_init_msg(struct nlmsg *msg, unsigned int ifindex, uint16_t type, uint16_t flags)
+tc_init_msg(struct tap_nlmsg *msg, unsigned int ifindex, uint16_t type, uint16_t flags)
 {
 	struct nlmsghdr *n = &msg->nh;
 
@@ -72,7 +72,7 @@ tc_init_msg(struct nlmsg *msg, unsigned int ifindex, uint16_t type, uint16_t fla
 static int
 qdisc_del(int nlsk_fd, unsigned int ifindex, struct qdisc *qinfo)
 {
-	struct nlmsg msg;
+	struct tap_nlmsg msg;
 	int fd = 0;
 
 	tc_init_msg(&msg, ifindex, RTM_DELQDISC, 0);
@@ -117,7 +117,7 @@ int
 qdisc_add_multiq(int nlsk_fd, unsigned int ifindex)
 {
 	struct tc_multiq_qopt opt = {0};
-	struct nlmsg msg;
+	struct tap_nlmsg msg;
 
 	tc_init_msg(&msg, ifindex, RTM_NEWQDISC,
 		    NLM_F_REQUEST | NLM_F_ACK | NLM_F_EXCL | NLM_F_CREATE);
@@ -146,7 +146,7 @@ qdisc_add_multiq(int nlsk_fd, unsigned int ifindex)
 int
 qdisc_add_ingress(int nlsk_fd, unsigned int ifindex)
 {
-	struct nlmsg msg;
+	struct tap_nlmsg msg;
 
 	tc_init_msg(&msg, ifindex, RTM_NEWQDISC,
 		    NLM_F_REQUEST | NLM_F_ACK | NLM_F_EXCL | NLM_F_CREATE);
@@ -211,7 +211,7 @@ static int
 qdisc_iterate(int nlsk_fd, unsigned int ifindex,
 	      int (*callback)(struct nlmsghdr *, void *), void *arg)
 {
-	struct nlmsg msg;
+	struct tap_nlmsg msg;
 	struct list_args args = {
 		.nlsk_fd = nlsk_fd,
 		.ifindex = ifindex,
diff --git a/drivers/net/tap/tap_tcmsgs.h b/drivers/net/tap/tap_tcmsgs.h
index 9411626661..70e97e2b62 100644
--- a/drivers/net/tap/tap_tcmsgs.h
+++ b/drivers/net/tap/tap_tcmsgs.h
@@ -24,7 +24,7 @@
 
 #define MULTIQ_MAJOR_HANDLE (1 << 16)
 
-void tc_init_msg(struct nlmsg *msg, unsigned int ifindex, uint16_t type,
+void tc_init_msg(struct tap_nlmsg *msg, unsigned int ifindex, uint16_t type,
 		 uint16_t flags);
 int qdisc_list(int nlsk_fd, unsigned int ifindex);
 int qdisc_flush(int nlsk_fd, unsigned int ifindex);
-- 
2.45.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4] net/tap: do not use opaque parameter in convert
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 1/4] net/tap: replace rte_memcpy Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 2/4] net/tap: rename struct nlmsg to tap_nlmsg Stephen Hemminger
@ 2024-10-28 16:32 ` Stephen Hemminger
  2024-10-28 16:32 ` [PATCH 4/4] net/tap: do not use rte_malloc for process private data Stephen Hemminger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2024-10-28 16:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The function to convert from internal representation to
netlink was using a void pointer when both caller and callee
were using same structure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/tap_flow.c | 47 +++++++++++++-------------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 6564583174..c0e44bb1a7 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -74,12 +74,12 @@ struct action_data {
 	};
 };
 
-static int tap_flow_create_eth(const struct rte_flow_item *item, void *data);
-static int tap_flow_create_vlan(const struct rte_flow_item *item, void *data);
-static int tap_flow_create_ipv4(const struct rte_flow_item *item, void *data);
-static int tap_flow_create_ipv6(const struct rte_flow_item *item, void *data);
-static int tap_flow_create_udp(const struct rte_flow_item *item, void *data);
-static int tap_flow_create_tcp(const struct rte_flow_item *item, void *data);
+static int tap_flow_create_eth(const struct rte_flow_item *item, struct convert_data *info);
+static int tap_flow_create_vlan(const struct rte_flow_item *item, struct convert_data *info);
+static int tap_flow_create_ipv4(const struct rte_flow_item *item, struct convert_data *info);
+static int tap_flow_create_ipv6(const struct rte_flow_item *item, struct convert_data *info);
+static int tap_flow_create_udp(const struct rte_flow_item *item, struct convert_data *info);
+static int tap_flow_create_tcp(const struct rte_flow_item *item, struct convert_data *info);
 static int
 tap_flow_validate(struct rte_eth_dev *dev,
 		  const struct rte_flow_attr *attr,
@@ -139,19 +139,10 @@ struct tap_flow_items {
 	 * along with the item.
 	 */
 	const void *default_mask;
-	/**
-	 * Conversion function from rte_flow to netlink attributes.
-	 *
-	 * @param item
-	 *   rte_flow item to convert.
-	 * @param data
-	 *   Internal structure to store the conversion.
-	 *
-	 * @return
-	 *   0 on success, negative value otherwise.
-	 */
-	int (*convert)(const struct rte_flow_item *item, void *data);
-	/** List of possible following items.  */
+	/* Conversion function from rte_flow to netlink attributes. */
+	int (*convert)(const struct rte_flow_item *item, struct convert_data *info);
+
+	/* List of possible following items.  */
 	const enum rte_flow_item_type *const items;
 };
 
@@ -417,9 +408,8 @@ static struct remote_rule implicit_rte_flows[TAP_REMOTE_MAX_IDX] = {
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_eth(const struct rte_flow_item *item, void *data)
+tap_flow_create_eth(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_eth *spec = item->spec;
 	const struct rte_flow_item_eth *mask = item->mask;
 	struct rte_flow *flow = info->flow;
@@ -471,9 +461,8 @@ tap_flow_create_eth(const struct rte_flow_item *item, void *data)
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_vlan(const struct rte_flow_item *item, void *data)
+tap_flow_create_vlan(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_vlan *spec = item->spec;
 	const struct rte_flow_item_vlan *mask = item->mask;
 	struct rte_flow *flow = info->flow;
@@ -531,9 +520,8 @@ tap_flow_create_vlan(const struct rte_flow_item *item, void *data)
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_ipv4(const struct rte_flow_item *item, void *data)
+tap_flow_create_ipv4(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_ipv4 *spec = item->spec;
 	const struct rte_flow_item_ipv4 *mask = item->mask;
 	struct rte_flow *flow = info->flow;
@@ -586,9 +574,8 @@ tap_flow_create_ipv4(const struct rte_flow_item *item, void *data)
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_ipv6(const struct rte_flow_item *item, void *data)
+tap_flow_create_ipv6(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_ipv6 *spec = item->spec;
 	const struct rte_flow_item_ipv6 *mask = item->mask;
 	struct rte_flow *flow = info->flow;
@@ -642,9 +629,8 @@ tap_flow_create_ipv6(const struct rte_flow_item *item, void *data)
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_udp(const struct rte_flow_item *item, void *data)
+tap_flow_create_udp(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_udp *spec = item->spec;
 	const struct rte_flow_item_udp *mask = item->mask;
 	struct rte_flow *flow = info->flow;
@@ -688,9 +674,8 @@ tap_flow_create_udp(const struct rte_flow_item *item, void *data)
  *   0 if checks are alright, -1 otherwise.
  */
 static int
-tap_flow_create_tcp(const struct rte_flow_item *item, void *data)
+tap_flow_create_tcp(const struct rte_flow_item *item, struct convert_data *info)
 {
-	struct convert_data *info = (struct convert_data *)data;
 	const struct rte_flow_item_tcp *spec = item->spec;
 	const struct rte_flow_item_tcp *mask = item->mask;
 	struct rte_flow *flow = info->flow;
-- 
2.45.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] net/tap: do not use rte_malloc for process private data
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
                   ` (2 preceding siblings ...)
  2024-10-28 16:32 ` [PATCH 3/4] net/tap: do not use opaque parameter in convert Stephen Hemminger
@ 2024-10-28 16:32 ` Stephen Hemminger
  2024-10-29  7:39 ` [PATCH 0/4] net/tap: minor cleanups Morten Brørup
  2024-11-02  1:40 ` Ferruh Yigit
  5 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2024-10-28 16:32 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The process private data is local to the process and does not
have to come from global shared memory. Using malloc() gets more
coverage with tools and has better caching.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/rte_eth_tap.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index da45610665..650ddbd706 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1109,7 +1109,8 @@ tap_dev_close(struct rte_eth_dev *dev)
 	struct pmd_process_private *process_private = dev->process_private;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		rte_free(dev->process_private);
+		free(dev->process_private);
+		dev->process_private = NULL;
 		if (tap_devices_count == 1)
 			rte_mp_action_unregister(TAP_MP_REQ_START_RXTX);
 		tap_devices_count--;
@@ -1170,7 +1171,9 @@ tap_dev_close(struct rte_eth_dev *dev)
 		close(internals->ioctl_sock);
 		internals->ioctl_sock = -1;
 	}
-	rte_free(dev->process_private);
+	free(dev->process_private);
+	dev->process_private = NULL;
+
 	if (tap_devices_count == 1)
 		rte_mp_action_unregister(TAP_MP_KEY);
 	tap_devices_count--;
@@ -1923,14 +1926,13 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, const char *tap_name,
 		goto error_exit_nodev;
 	}
 
-	process_private = (struct pmd_process_private *)
-		rte_zmalloc_socket(tap_name, sizeof(struct pmd_process_private),
-			RTE_CACHE_LINE_SIZE, dev->device->numa_node);
-
+	process_private = malloc(sizeof(struct pmd_process_private));
 	if (process_private == NULL) {
 		TAP_LOG(ERR, "Failed to alloc memory for process private");
 		return -1;
 	}
+	memset(process_private, 0, sizeof(struct pmd_process_private));
+
 	pmd = dev->data->dev_private;
 	dev->process_private = process_private;
 	pmd->dev = dev;
@@ -2435,16 +2437,13 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
 			TAP_LOG(ERR, "Primary process is missing");
 			return -1;
 		}
-		eth_dev->process_private = (struct pmd_process_private *)
-			rte_zmalloc_socket(name,
-				sizeof(struct pmd_process_private),
-				RTE_CACHE_LINE_SIZE,
-				eth_dev->device->numa_node);
+		eth_dev->process_private = malloc(sizeof(struct pmd_process_private));
 		if (eth_dev->process_private == NULL) {
 			TAP_LOG(ERR,
 				"Failed to alloc memory for process private");
 			return -1;
 		}
+		memset(eth_dev->process_private, 0, sizeof(struct pmd_process_private));
 
 		ret = tap_mp_attach_queues(name, eth_dev);
 		if (ret != 0)
-- 
2.45.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 0/4] net/tap: minor cleanups
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
                   ` (3 preceding siblings ...)
  2024-10-28 16:32 ` [PATCH 4/4] net/tap: do not use rte_malloc for process private data Stephen Hemminger
@ 2024-10-29  7:39 ` Morten Brørup
  2024-11-02  1:40 ` Ferruh Yigit
  5 siblings, 0 replies; 7+ messages in thread
From: Morten Brørup @ 2024-10-29  7:39 UTC (permalink / raw)
  To: Stephen Hemminger, dev

For the series,
Acked-by: Morten Brørup <mb@smartsharesystems.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] net/tap: minor cleanups
  2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
                   ` (4 preceding siblings ...)
  2024-10-29  7:39 ` [PATCH 0/4] net/tap: minor cleanups Morten Brørup
@ 2024-11-02  1:40 ` Ferruh Yigit
  5 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2024-11-02  1:40 UTC (permalink / raw)
  To: Stephen Hemminger, dev

On 10/28/2024 4:32 PM, Stephen Hemminger wrote:
> These are some minor things in tap device that can be easily fixed.
> Not urgent can wait until 25.03 release but submitting now for review.
> 
> Stephen Hemminger (4):
>   net/tap: replace rte_memcpy
>   net/tap: rename struct nlmsg to tap_nlmsg
>   net/tap: do not use opaque parameter in convert
>   net/tap: do not use rte_malloc for process private data
> 

For the series,
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>


Series applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-11-02  1:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-28 16:32 [PATCH 0/4] net/tap: minor cleanups Stephen Hemminger
2024-10-28 16:32 ` [PATCH 1/4] net/tap: replace rte_memcpy Stephen Hemminger
2024-10-28 16:32 ` [PATCH 2/4] net/tap: rename struct nlmsg to tap_nlmsg Stephen Hemminger
2024-10-28 16:32 ` [PATCH 3/4] net/tap: do not use opaque parameter in convert Stephen Hemminger
2024-10-28 16:32 ` [PATCH 4/4] net/tap: do not use rte_malloc for process private data Stephen Hemminger
2024-10-29  7:39 ` [PATCH 0/4] net/tap: minor cleanups Morten Brørup
2024-11-02  1:40 ` Ferruh Yigit

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).