[RFC,02/14] net: add rte prefix to arp defines

Message ID 20181024081833.21432-3-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series prefix network structures |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Olivier Matz Oct. 24, 2018, 8:18 a.m. UTC
  Add 'RTE_' prefix to defines:
- rename ARP_HRD_ETHER as RTE_ARP_HRD_ETHER.
- rename ARP_OP_REQUEST as RTE_ARP_OP_REQUEST.
- rename ARP_OP_REPLY as RTE_ARP_OP_REPLY.
- rename ARP_OP_REVREQUEST as RTE_ARP_OP_REVREQUEST.
- rename ARP_OP_REVREPLY as RTE_ARP_OP_REVREPLY.
- rename ARP_OP_INVREQUEST as RTE_ARP_OP_INVREQUEST.
- rename ARP_OP_INVREPLY as RTE_ARP_OP_INVREPLY.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/icmpecho.c                | 18 +++++++++---------
 drivers/net/bonding/rte_eth_bond_alb.c |  8 ++++----
 drivers/net/bonding/rte_eth_bond_pmd.c | 12 ++++++------
 examples/bond/main.c                   |  8 ++++----
 lib/librte_net/rte_arp.c               |  4 ++--
 lib/librte_net/rte_arp.h               | 14 +++++++-------
 test/test/packet_burst_generator.c     |  2 +-
 test/test/test_link_bonding.c          | 18 +++++++++---------
 8 files changed, 42 insertions(+), 42 deletions(-)
  

Comments

Wiles, Keith Oct. 24, 2018, 2:53 p.m. UTC | #1
> On Oct 24, 2018, at 1:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> Add 'RTE_' prefix to defines:
> - rename ARP_HRD_ETHER as RTE_ARP_HRD_ETHER.
> - rename ARP_OP_REQUEST as RTE_ARP_OP_REQUEST.
> - rename ARP_OP_REPLY as RTE_ARP_OP_REPLY.
> - rename ARP_OP_REVREQUEST as RTE_ARP_OP_REVREQUEST.
> - rename ARP_OP_REVREPLY as RTE_ARP_OP_REVREPLY.
> - rename ARP_OP_INVREQUEST as RTE_ARP_OP_INVREQUEST.
> - rename ARP_OP_INVREPLY as RTE_ARP_OP_INVREPLY.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Were these conflicting with external headers ?

If not it maybe reasonable to keep the old defines for a release or two and then deprecate the old ones by defining the old defines in terms of the new defines. This way we can support the old ones for a while then state they will be deprecated later.

Trying to make it easier for developers to update code. What do you think

Regards,
Keith
  
Olivier Matz Oct. 26, 2018, 7:25 a.m. UTC | #2
Hi,

On Wed, Oct 24, 2018 at 02:53:25PM +0000, Wiles, Keith wrote:
> 
> 
> > On Oct 24, 2018, at 1:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> > 
> > Add 'RTE_' prefix to defines:
> > - rename ARP_HRD_ETHER as RTE_ARP_HRD_ETHER.
> > - rename ARP_OP_REQUEST as RTE_ARP_OP_REQUEST.
> > - rename ARP_OP_REPLY as RTE_ARP_OP_REPLY.
> > - rename ARP_OP_REVREQUEST as RTE_ARP_OP_REVREQUEST.
> > - rename ARP_OP_REVREPLY as RTE_ARP_OP_REVREPLY.
> > - rename ARP_OP_INVREQUEST as RTE_ARP_OP_INVREQUEST.
> > - rename ARP_OP_INVREPLY as RTE_ARP_OP_INVREPLY.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Were these conflicting with external headers ?
> 
> If not it maybe reasonable to keep the old defines for a release or two and then deprecate the old ones by defining the old defines in terms of the new defines. This way we can support the old ones for a while then state they will be deprecated later.
> 
> Trying to make it easier for developers to update code. What do you think

Providing a compat layer would be helpful for sure. But I don't think
it is possible for everything. Please see my answer to Bruce:
https://mails.dpdk.org/archives/dev/2018-October/117255.html

Olivier
  

Patch

diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c
index 34f68d73b..062e3ea43 100644
--- a/app/test-pmd/icmpecho.c
+++ b/app/test-pmd/icmpecho.c
@@ -39,17 +39,17 @@  static const char *
 arp_opcode_name(uint16_t arp_opcode)
 {
 	switch (arp_opcode ) {
-	case ARP_OP_REQUEST:
+	case RTE_ARP_OP_REQUEST:
 		return "ARP Request";
-	case ARP_OP_REPLY:
+	case RTE_ARP_OP_REPLY:
 		return "ARP Reply";
-	case ARP_OP_REVREQUEST:
+	case RTE_ARP_OP_REVREQUEST:
 		return "Reverse ARP Request";
-	case ARP_OP_REVREPLY:
+	case RTE_ARP_OP_REVREPLY:
 		return "Reverse ARP Reply";
-	case ARP_OP_INVREQUEST:
+	case RTE_ARP_OP_INVREQUEST:
 		return "Peer Identify Request";
-	case ARP_OP_INVREPLY:
+	case RTE_ARP_OP_INVREPLY:
 		return "Peer Identify Reply";
 	default:
 		break;
@@ -359,7 +359,7 @@  reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 				       arp_opcode_name(arp_opcode));
 			}
 			if ((RTE_BE_TO_CPU_16(arp_h->arp_hardware) !=
-			     ARP_HRD_ETHER) ||
+			     RTE_ARP_HRD_ETHER) ||
 			    (arp_protocol != ETHER_TYPE_IPv4) ||
 			    (arp_h->arp_hlen != 6) ||
 			    (arp_h->arp_plen != 4)
@@ -381,7 +381,7 @@  reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 				ipv4_addr_dump(" tip=", ip_addr);
 				printf("\n");
 			}
-			if (arp_opcode != ARP_OP_REQUEST) {
+			if (arp_opcode != RTE_ARP_OP_REQUEST) {
 				rte_pktmbuf_free(pkt);
 				continue;
 			}
@@ -396,7 +396,7 @@  reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 			ether_addr_copy(&ports[fs->tx_port].eth_addr,
 					&eth_h->s_addr);
 
-			arp_h->arp_opcode = rte_cpu_to_be_16(ARP_OP_REPLY);
+			arp_h->arp_opcode = rte_cpu_to_be_16(RTE_ARP_OP_REPLY);
 			ether_addr_copy(&arp_h->arp_data.arp_tha, &eth_addr);
 			ether_addr_copy(&arp_h->arp_data.arp_sha, &arp_h->arp_data.arp_tha);
 			ether_addr_copy(&eth_h->s_addr, &arp_h->arp_data.arp_sha);
diff --git a/drivers/net/bonding/rte_eth_bond_alb.c b/drivers/net/bonding/rte_eth_bond_alb.c
index b2aeb3ca9..7ae240345 100644
--- a/drivers/net/bonding/rte_eth_bond_alb.c
+++ b/drivers/net/bonding/rte_eth_bond_alb.c
@@ -84,7 +84,7 @@  void bond_mode_alb_arp_recv(struct ether_hdr *eth_h, uint16_t offset,
 	arp = (struct rte_arp_hdr *) ((char *) (eth_h + 1) + offset);
 
 	/* ARP Requests are forwarded to the application with no changes */
-	if (arp->arp_opcode != rte_cpu_to_be_16(ARP_OP_REPLY))
+	if (arp->arp_opcode != rte_cpu_to_be_16(RTE_ARP_OP_REPLY))
 		return;
 
 	/* From now on, we analyze only ARP Reply packets */
@@ -150,7 +150,7 @@  bond_mode_alb_arp_xmit(struct ether_hdr *eth_h, uint16_t offset,
 	client_info = &hash_table[hash_index];
 
 	rte_spinlock_lock(&internals->mode6.lock);
-	if (arp->arp_opcode == rte_cpu_to_be_16(ARP_OP_REPLY)) {
+	if (arp->arp_opcode == rte_cpu_to_be_16(RTE_ARP_OP_REPLY)) {
 		if (client_info->in_use) {
 			if (client_info->app_ip == arp->arp_data.arp_sip &&
 				client_info->cli_ip == arp->arp_data.arp_tip) {
@@ -220,11 +220,11 @@  bond_mode_alb_arp_upd(struct client_data *client_info,
 	ether_addr_copy(&client_info->cli_mac, &arp_h->arp_data.arp_tha);
 	arp_h->arp_data.arp_tip = client_info->cli_ip;
 
-	arp_h->arp_hardware = rte_cpu_to_be_16(ARP_HRD_ETHER);
+	arp_h->arp_hardware = rte_cpu_to_be_16(RTE_ARP_HRD_ETHER);
 	arp_h->arp_protocol = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 	arp_h->arp_hlen = ETHER_ADDR_LEN;
 	arp_h->arp_plen = sizeof(uint32_t);
-	arp_h->arp_opcode = rte_cpu_to_be_16(ARP_OP_REPLY);
+	arp_h->arp_opcode = rte_cpu_to_be_16(RTE_ARP_OP_REPLY);
 
 	slave_idx = client_info->slave_idx;
 	rte_spinlock_unlock(&internals->mode6.lock);
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index a86a74a8d..52724640f 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -488,25 +488,25 @@  static void
 arp_opcode_name(uint16_t arp_opcode, char *buf)
 {
 	switch (arp_opcode) {
-	case ARP_OP_REQUEST:
+	case RTE_ARP_OP_REQUEST:
 		snprintf(buf, sizeof("ARP Request"), "%s", "ARP Request");
 		return;
-	case ARP_OP_REPLY:
+	case RTE_ARP_OP_REPLY:
 		snprintf(buf, sizeof("ARP Reply"), "%s", "ARP Reply");
 		return;
-	case ARP_OP_REVREQUEST:
+	case RTE_ARP_OP_REVREQUEST:
 		snprintf(buf, sizeof("Reverse ARP Request"), "%s",
 				"Reverse ARP Request");
 		return;
-	case ARP_OP_REVREPLY:
+	case RTE_ARP_OP_REVREPLY:
 		snprintf(buf, sizeof("Reverse ARP Reply"), "%s",
 				"Reverse ARP Reply");
 		return;
-	case ARP_OP_INVREQUEST:
+	case RTE_ARP_OP_INVREQUEST:
 		snprintf(buf, sizeof("Peer Identify Request"), "%s",
 				"Peer Identify Request");
 		return;
-	case ARP_OP_INVREPLY:
+	case RTE_ARP_OP_INVREPLY:
 		snprintf(buf, sizeof("Peer Identify Reply"), "%s",
 				"Peer Identify Reply");
 		return;
diff --git a/examples/bond/main.c b/examples/bond/main.c
index d428c405f..2fe5a9319 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -369,8 +369,8 @@  static int lcore_main(__attribute__((unused)) void *arg1)
 				}
 				arp_hdr = (struct rte_arp_hdr *)((char *)(eth_hdr + 1) + offset);
 				if (arp_hdr->arp_data.arp_tip == bond_ip) {
-					if (arp_hdr->arp_opcode == rte_cpu_to_be_16(ARP_OP_REQUEST)) {
-						arp_hdr->arp_opcode = rte_cpu_to_be_16(ARP_OP_REPLY);
+					if (arp_hdr->arp_opcode == rte_cpu_to_be_16(RTE_ARP_OP_REQUEST)) {
+						arp_hdr->arp_opcode = rte_cpu_to_be_16(RTE_ARP_OP_REPLY);
 						/* Switch src and dst data and set bonding MAC */
 						ether_addr_copy(&eth_hdr->s_addr, &eth_hdr->d_addr);
 						rte_eth_macaddr_get(BOND_PORT, &eth_hdr->s_addr);
@@ -464,11 +464,11 @@  static void cmd_obj_send_parsed(void *parsed_result,
 	eth_hdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_ARP);
 
 	arp_hdr = (struct rte_arp_hdr *)((char *)eth_hdr + sizeof(struct ether_hdr));
-	arp_hdr->arp_hardware = rte_cpu_to_be_16(ARP_HRD_ETHER);
+	arp_hdr->arp_hardware = rte_cpu_to_be_16(RTE_ARP_HRD_ETHER);
 	arp_hdr->arp_protocol = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 	arp_hdr->arp_hlen = ETHER_ADDR_LEN;
 	arp_hdr->arp_plen = sizeof(uint32_t);
-	arp_hdr->arp_opcode = rte_cpu_to_be_16(ARP_OP_REQUEST);
+	arp_hdr->arp_opcode = rte_cpu_to_be_16(RTE_ARP_OP_REQUEST);
 
 	rte_eth_macaddr_get(BOND_PORT, &arp_hdr->arp_data.arp_sha);
 	arp_hdr->arp_data.arp_sip = bond_ip;
diff --git a/lib/librte_net/rte_arp.c b/lib/librte_net/rte_arp.c
index c80ebc7e1..877874a5e 100644
--- a/lib/librte_net/rte_arp.c
+++ b/lib/librte_net/rte_arp.c
@@ -35,11 +35,11 @@  rte_net_make_rarp_packet(struct rte_mempool *mpool,
 
 	/* RARP header. */
 	rarp = (struct rte_arp_hdr *)(eth_hdr + 1);
-	rarp->arp_hardware = htons(ARP_HRD_ETHER);
+	rarp->arp_hardware = htons(RTE_ARP_HRD_ETHER);
 	rarp->arp_protocol = htons(ETHER_TYPE_IPv4);
 	rarp->arp_hlen = ETHER_ADDR_LEN;
 	rarp->arp_plen = 4;
-	rarp->arp_opcode  = htons(ARP_OP_REVREQUEST);
+	rarp->arp_opcode  = htons(RTE_ARP_OP_REVREQUEST);
 
 	ether_addr_copy(mac, &rarp->arp_data.arp_sha);
 	ether_addr_copy(mac, &rarp->arp_data.arp_tha);
diff --git a/lib/librte_net/rte_arp.h b/lib/librte_net/rte_arp.h
index c9b5cd49d..a94fa6a00 100644
--- a/lib/librte_net/rte_arp.h
+++ b/lib/librte_net/rte_arp.h
@@ -33,18 +33,18 @@  struct rte_arp_ipv4 {
  */
 struct rte_arp_hdr {
 	uint16_t arp_hardware;    /* format of hardware address */
-#define ARP_HRD_ETHER     1  /* ARP Ethernet address format */
+#define RTE_ARP_HRD_ETHER     1  /* ARP Ethernet address format */
 
 	uint16_t arp_protocol;    /* format of protocol address */
 	uint8_t  arp_hlen;    /* length of hardware address */
 	uint8_t  arp_plen;    /* length of protocol address */
 	uint16_t arp_opcode;     /* ARP opcode (command) */
-#define	ARP_OP_REQUEST    1 /* request to resolve address */
-#define	ARP_OP_REPLY      2 /* response to previous request */
-#define	ARP_OP_REVREQUEST 3 /* request proto addr given hardware */
-#define	ARP_OP_REVREPLY   4 /* response giving protocol address */
-#define	ARP_OP_INVREQUEST 8 /* request to identify peer */
-#define	ARP_OP_INVREPLY   9 /* response identifying peer */
+#define	RTE_ARP_OP_REQUEST    1 /* request to resolve address */
+#define	RTE_ARP_OP_REPLY      2 /* response to previous request */
+#define	RTE_ARP_OP_REVREQUEST 3 /* request proto addr given hardware */
+#define	RTE_ARP_OP_REVREPLY   4 /* response giving protocol address */
+#define	RTE_ARP_OP_INVREQUEST 8 /* request to identify peer */
+#define	RTE_ARP_OP_INVREPLY   9 /* response identifying peer */
 
 	struct rte_arp_ipv4 arp_data;
 } __attribute__((__packed__));
diff --git a/test/test/packet_burst_generator.c b/test/test/packet_burst_generator.c
index 0b12058bb..ccc0bd591 100644
--- a/test/test/packet_burst_generator.c
+++ b/test/test/packet_burst_generator.c
@@ -78,7 +78,7 @@  initialize_arp_header(struct rte_arp_hdr *arp_hdr, struct ether_addr *src_mac,
 		struct ether_addr *dst_mac, uint32_t src_ip, uint32_t dst_ip,
 		uint32_t opcode)
 {
-	arp_hdr->arp_hardware = rte_cpu_to_be_16(ARP_HRD_ETHER);
+	arp_hdr->arp_hardware = rte_cpu_to_be_16(RTE_ARP_HRD_ETHER);
 	arp_hdr->arp_protocol = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
 	arp_hdr->arp_hlen = ETHER_ADDR_LEN;
 	arp_hdr->arp_plen = sizeof(uint32_t);
diff --git a/test/test/test_link_bonding.c b/test/test/test_link_bonding.c
index db437a9c2..2c20f44ea 100644
--- a/test/test/test_link_bonding.c
+++ b/test/test/test_link_bonding.c
@@ -4497,7 +4497,7 @@  test_alb_change_mac_in_reply_sent(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &bond_mac, &client_mac, ip_host, ip_client1,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	rte_eth_tx_burst(test_params->bonded_port_id, 0, &pkt, 1);
 
 	pkt = rte_pktmbuf_alloc(test_params->mbuf_pool);
@@ -4507,7 +4507,7 @@  test_alb_change_mac_in_reply_sent(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &bond_mac, &client_mac, ip_host, ip_client2,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	rte_eth_tx_burst(test_params->bonded_port_id, 0, &pkt, 1);
 
 	pkt = rte_pktmbuf_alloc(test_params->mbuf_pool);
@@ -4517,7 +4517,7 @@  test_alb_change_mac_in_reply_sent(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &bond_mac, &client_mac, ip_host, ip_client3,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	rte_eth_tx_burst(test_params->bonded_port_id, 0, &pkt, 1);
 
 	pkt = rte_pktmbuf_alloc(test_params->mbuf_pool);
@@ -4527,7 +4527,7 @@  test_alb_change_mac_in_reply_sent(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &bond_mac, &client_mac, ip_host, ip_client4,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	rte_eth_tx_burst(test_params->bonded_port_id, 0, &pkt, 1);
 
 	slave_mac1 =
@@ -4610,7 +4610,7 @@  test_alb_reply_from_client(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &client_mac, &bond_mac, ip_client1, ip_host,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[0], &pkt,
 			1);
 
@@ -4621,7 +4621,7 @@  test_alb_reply_from_client(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &client_mac, &bond_mac, ip_client2, ip_host,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[0], &pkt,
 			1);
 
@@ -4632,7 +4632,7 @@  test_alb_reply_from_client(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &client_mac, &bond_mac, ip_client3, ip_host,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[0], &pkt,
 			1);
 
@@ -4643,7 +4643,7 @@  test_alb_reply_from_client(void)
 			0);
 	arp_pkt = (struct rte_arp_hdr *)((char *)eth_pkt + sizeof(struct ether_hdr));
 	initialize_arp_header(arp_pkt, &client_mac, &bond_mac, ip_client4, ip_host,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[0], &pkt,
 			1);
 
@@ -4742,7 +4742,7 @@  test_alb_receive_vlan_reply(void)
 	vlan_pkt->eth_proto = rte_cpu_to_be_16(ETHER_TYPE_ARP);
 	arp_pkt = (struct rte_arp_hdr *)((char *)(vlan_pkt + 1));
 	initialize_arp_header(arp_pkt, &client_mac, &bond_mac, ip_client1, ip_host,
-			ARP_OP_REPLY);
+			RTE_ARP_OP_REPLY);
 	virtual_ethdev_add_mbufs_to_rx_queue(test_params->slave_port_ids[0], &pkt,
 			1);