Re: [dpdk-dev] [PATCH v2] fix lacp check system address

2020-06-04 Thread podovinnikov

Hi Ferruh!

Unfortunately I didn't see any patch from this link

https://patchwork.dpdk.org/user/todo/dpdk/?series=8679

10.04.2020 13:24, Ferruh Yigit пишет:

On 4/10/2020 11:21 AM, Ferruh Yigit wrote:

On 4/10/2020 11:15 AM, Ferruh Yigit wrote:

On 11/26/2019 3:09 PM, podovinnikov wrote:

Hi Ferruh


This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
Linville).

Sorry about that. I have 2 patches ( one for af packet, one for bonding).

I am a little confused.



Can you please give more details, what is failing what is the impact etc.

In bond (lacp) we have several nics ( ports )

When we have negotiation with peer about what port we prefer,

we send information about what system we preferred in partner system
name field.

Peer also sends us what partner system name it prefer.

When we receive a message from it we must compare its preferred

system name with our system name, but not with our port mac address

In my test I have several problems with that

1. If master port (mac address same as system address)

shuts down (I have two ports) I loose connection

2. If secondary port (mac address not same as system address)

receives message before master port, my connection is not established.

Hi Vadim,

Thanks for the info and sorry for late response, it is sitting in backlog for a
long time now.

@Chas, do you have any objection on the fix, if not I am planning to merge it 
soon.

Thanks,
ferruh

cc'ed Xavier, he also has a bonding patch in the backlog waiting for review [1].


Vadim, Xavier,

Since you both send bonding patches, I assume you both know about bonding at
some level, at least more than me, so would you mind reviewing eachother's
patch? So both can go it.

+ other email address of the Xavier.


Thanks,
ferruh


[1]
https://patchwork.dpdk.org/user/todo/dpdk/?series=8679



26.11.2019 16:27, Ferruh Yigit пишет:

Hi Vadim,

On 11/26/2019 11:55 AM, Vadim wrote:

Signed-off-by: Vadim 

Can you please provide full "Name Surname " signature?


This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
Linville).


fix lacp check system address

Can you please give more details, what is failing what is the impact etc.


---
   drivers/net/bonding/rte_eth_bond_8023ad.c | 17 -
   1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c 
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index b77a37ddb..d4dda790a 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -792,18 +792,33 @@ rx_machine_update(struct bond_dev_private *internals, 
uint16_t slave_id,
struct rte_mbuf *lacp_pkt) {
struct lacpdu_header *lacp;
struct lacpdu_actor_partner_params *partner;
+   struct port *port, *agg;
   
   	if (lacp_pkt != NULL) {

lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
RTE_ASSERT(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
   
   		partner = &lacp->lacpdu.partner;

+   port = &bond_mode_8023ad_ports[slave_id];
+   agg = &bond_mode_8023ad_ports[port->aggregator_port_id];
+
if (rte_is_same_ether_addr(&partner->port_params.system,
-   &internals->mode4.mac_addr)) {
+   &agg->actor.system)) {
/* This LACP frame is sending to the bonding port
 * so pass it to rx_machine.
 */
rx_machine(internals, slave_id, &lacp->lacpdu);
+   } else {
+   char preferred_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+   char self_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+   rte_ether_format_addr(preferred_system_name,
+   RTE_ETHER_ADDR_FMT_SIZE,
+   &partner->port_params.system);
+   rte_ether_format_addr(self_system_name,
+   RTE_ETHER_ADDR_FMT_SIZE, &agg->actor.system);
+   MODE4_DEBUG("preferred partner system %s not equal "
+   "self system: %s\n",
+   preferred_system_name, self_system_name);
}
rte_pktmbuf_free(lacp_pkt);
} else



Re: [dpdk-dev] [PATCH v2] fix lacp check system address

2019-11-26 Thread podovinnikov

Hi Ferruh


This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
Linville).

Sorry about that. I have 2 patches ( one for af packet, one for bonding).

I am a little confused.



Can you please give more details, what is failing what is the impact etc.


In bond (lacp) we have several nics ( ports )

When we have negotiation with peer about what port we prefer,

we send information about what system we preferred in partner system 
name field.


Peer also sends us what partner system name it prefer.

When we receive a message from it we must compare its preferred

system name with our system name, but not with our port mac address

In my test I have several problems with that

1. If master port (mac address same as system address)

shuts down (I have two ports) I loose connection

2. If secondary port (mac address not same as system address)

receives message before master port, my connection is not established.


26.11.2019 16:27, Ferruh Yigit пишет:

Hi Vadim,

On 11/26/2019 11:55 AM, Vadim wrote:

Signed-off-by: Vadim 

Can you please provide full "Name Surname " signature?


This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
Linville).


fix lacp check system address

Can you please give more details, what is failing what is the impact etc.


---
  drivers/net/bonding/rte_eth_bond_8023ad.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c 
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index b77a37ddb..d4dda790a 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -792,18 +792,33 @@ rx_machine_update(struct bond_dev_private *internals, 
uint16_t slave_id,
struct rte_mbuf *lacp_pkt) {
struct lacpdu_header *lacp;
struct lacpdu_actor_partner_params *partner;
+   struct port *port, *agg;
  
  	if (lacp_pkt != NULL) {

lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
RTE_ASSERT(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
  
  		partner = &lacp->lacpdu.partner;

+   port = &bond_mode_8023ad_ports[slave_id];
+   agg = &bond_mode_8023ad_ports[port->aggregator_port_id];
+
if (rte_is_same_ether_addr(&partner->port_params.system,
-   &internals->mode4.mac_addr)) {
+   &agg->actor.system)) {
/* This LACP frame is sending to the bonding port
 * so pass it to rx_machine.
 */
rx_machine(internals, slave_id, &lacp->lacpdu);
+   } else {
+   char preferred_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+   char self_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+   rte_ether_format_addr(preferred_system_name,
+   RTE_ETHER_ADDR_FMT_SIZE,
+   &partner->port_params.system);
+   rte_ether_format_addr(self_system_name,
+   RTE_ETHER_ADDR_FMT_SIZE, &agg->actor.system);
+   MODE4_DEBUG("preferred partner system %s not equal "
+   "self system: %s\n",
+   preferred_system_name, self_system_name);
}
rte_pktmbuf_free(lacp_pkt);
} else



[dpdk-dev] [PATCH] fix memif does not free resources

2020-04-04 Thread Vadim Podovinnikov
Signed-off-by: Vadim Podovinnikov 
---
 drivers/net/memif/rte_eth_memif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/memif/rte_eth_memif.c 
b/drivers/net/memif/rte_eth_memif.c
index 81d71c53a..653ffa9b8 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1510,7 +1510,7 @@ memif_create(struct rte_vdev_device *vdev, enum 
memif_role_t role,
}
 
 
-   eth_dev->data->dev_flags &= RTE_ETH_DEV_CLOSE_REMOVE;
+   eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
 
rte_eth_dev_probing_finish(eth_dev);
 
-- 
2.26.0



[dpdk-dev] [PATCH v1] fix memif does not free resources

2020-04-04 Thread Vadim Podovinnikov
Signed-off-by: Vadim Podovinnikov 
---
 drivers/net/memif/rte_eth_memif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/memif/rte_eth_memif.c 
b/drivers/net/memif/rte_eth_memif.c
index 81d71c53a..653ffa9b8 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1510,7 +1510,7 @@ memif_create(struct rte_vdev_device *vdev, enum 
memif_role_t role,
}
 
 
-   eth_dev->data->dev_flags &= RTE_ETH_DEV_CLOSE_REMOVE;
+   eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
 
rte_eth_dev_probing_finish(eth_dev);
 
-- 
2.26.0



[dpdk-dev] [PATCH v3] add drop statistic for af_packet

2019-11-26 Thread Vadim Podovinnikov
From: Vadim 

add drop statistic for af_packet

Signed-off-by: Vadim Podovinnikov 
---
 drivers/net/af_packet/rte_eth_af_packet.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index f5806bf42..eee0fbce2 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -327,8 +327,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *igb_stats)
 {
unsigned i, imax;
unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
-   unsigned long rx_bytes_total = 0, tx_bytes_total = 0;
+   unsigned long rx_bytes_total = 0, tx_bytes_total = 0, rx_drop = 0;
const struct pmd_internals *internal = dev->data->dev_private;
+   socklen_t sock_len = sizeof(struct tpacket_stats);
+   struct tpacket_stats st;
 
imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
@@ -337,6 +339,12 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *igb_stats)
igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
rx_total += igb_stats->q_ipackets[i];
rx_bytes_total += igb_stats->q_ibytes[i];
+
+   memset(&st, 0, sock_len);
+   int rc = getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET,
+   PACKET_STATISTICS, &st, &sock_len);
+   if (rc == 0)
+   rx_drop += st.tp_drops;
}
 
imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
@@ -349,6 +357,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats 
*igb_stats)
tx_bytes_total += igb_stats->q_obytes[i];
}
 
+   igb_stats->imissed = rx_drop;
igb_stats->ipackets = rx_total;
igb_stats->ibytes = rx_bytes_total;
igb_stats->opackets = tx_total;
-- 
2.24.0



[dpdk-dev] [PATCH v4] add drop statistic for af_packet

2019-11-29 Thread Vadim Podovinnikov
Signed-off-by: Vadim Podovinnikov 
---
 drivers/net/af_packet/rte_eth_af_packet.c | 33 +--
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c 
b/drivers/net/af_packet/rte_eth_af_packet.c
index eee0fbce2..2aa7c0fcc 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -52,6 +52,7 @@ struct pkt_rx_queue {
 
volatile unsigned long rx_pkts;
volatile unsigned long rx_bytes;
+   volatile unsigned long rx_drop;
 };
 
 struct pkt_tx_queue {
@@ -322,6 +323,25 @@ eth_dev_info(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
return 0;
 }
 
+static void
+fill_eth_drop_stats(struct rte_eth_dev *dev)
+{
+   unsigned int i, imax;
+   struct pmd_internals *internal = dev->data->dev_private;
+   socklen_t sock_len = sizeof(struct tpacket_stats);
+   struct tpacket_stats st;
+
+   imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
+   internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
+   for (i = 0; i < imax; i++) {
+   memset(&st, 0, sock_len);
+   int rc = getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET,
+   PACKET_STATISTICS, &st, &sock_len);
+   if (rc == 0)
+   internal->rx_queue[i].rx_drop += st.tp_drops;
+   }
+}
+
 static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
 {
@@ -329,22 +349,18 @@ eth_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *igb_stats)
unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
unsigned long rx_bytes_total = 0, tx_bytes_total = 0, rx_drop = 0;
const struct pmd_internals *internal = dev->data->dev_private;
-   socklen_t sock_len = sizeof(struct tpacket_stats);
-   struct tpacket_stats st;
+
+   fill_eth_drop_stats(dev);
 
imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
for (i = 0; i < imax; i++) {
igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
+   igb_stats->q_errors[i] = internal->rx_queue[i].rx_drop;
rx_total += igb_stats->q_ipackets[i];
rx_bytes_total += igb_stats->q_ibytes[i];
-
-   memset(&st, 0, sock_len);
-   int rc = getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET,
-   PACKET_STATISTICS, &st, &sock_len);
-   if (rc == 0)
-   rx_drop += st.tp_drops;
+   rx_drop += igb_stats->q_errors[i];
}
 
imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
@@ -375,6 +391,7 @@ eth_stats_reset(struct rte_eth_dev *dev)
for (i = 0; i < internal->nb_queues; i++) {
internal->rx_queue[i].rx_pkts = 0;
internal->rx_queue[i].rx_bytes = 0;
+   internal->rx_queue[i].rx_drop = 0;
}
 
for (i = 0; i < internal->nb_queues; i++) {
-- 
2.24.0