[1/2] net/bonding: support private dump ops

Message ID 20221205081051.25905-2-fengchengwen@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support device private dump |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

fengchengwen Dec. 5, 2022, 8:10 a.m. UTC
  This patch implements eth_dev_priv_dump ops which could enhance the
debug capability.

The dump output is similar to testpmd command
"show bonding config [port]".

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 103 ++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)
  

Comments

humin (Q) Dec. 6, 2022, 2:01 a.m. UTC | #1
Hi,

在 2022/12/5 16:10, Chengwen Feng 写道:
> This patch implements eth_dev_priv_dump ops which could enhance the
> debug capability.
>
> The dump output is similar to testpmd command
> "show bonding config [port]".
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   drivers/net/bonding/rte_eth_bond_pmd.c | 103 ++++++++++++++++++++++++-
>   1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b9bcebc6cb..80fb2dc462 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -3329,6 +3329,106 @@ bond_ethdev_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
>   	rte_spinlock_unlock(&internals->lock);
>   }
>   
> +static const char *
> +bond_mode_name(uint8_t mode)
> +{
> +	switch (mode) {
> +	case BONDING_MODE_ROUND_ROBIN:
> +		return "ROUND_ROBIN";
> +	case BONDING_MODE_ACTIVE_BACKUP:
> +		return "ACTIVE_BACKUP";
> +	case BONDING_MODE_BALANCE:
> +		return "BALANCE";
> +	case BONDING_MODE_BROADCAST:
> +		return "BROADCAST";
> +	case BONDING_MODE_8023AD:
> +		return "8023AD";
> +	case BONDING_MODE_TLB:
> +		return "TLB";
> +	case BONDING_MODE_ALB:
> +		return "ALB";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static int
> +bond_ethdev_priv_dump(struct rte_eth_dev *dev, FILE *f)
> +{
> +	struct bond_dev_private instant_priv;
> +	const struct bond_dev_private *internals = &instant_priv;
> +	int bonding_mode;
> +	int i;
> +
> +	/* Obtain a instance of dev_private to prevent data from being modified. */
> +	memcpy(&instant_priv, dev->data->dev_private, sizeof(struct bond_dev_private));
> +	bonding_mode = internals->mode;
> +
> +	fprintf(f, "  - Dev basic:\n");
> +	fprintf(f, "\tBonding mode: %s(%d)\n", bond_mode_name(bonding_mode), bonding_mode);
data type of“bonding mode”does not match "mode".
> +
> +	if (bonding_mode == BONDING_MODE_BALANCE ||
> +		bonding_mode == BONDING_MODE_8023AD) {
> +		fprintf(f, "\tBalance Xmit Policy: ");
> +		switch (internals->balance_xmit_policy) {
> +		case BALANCE_XMIT_POLICY_LAYER2:
> +			fprintf(f, "BALANCE_XMIT_POLICY_LAYER2");
> +			break;
> +		case BALANCE_XMIT_POLICY_LAYER23:
> +			fprintf(f, "BALANCE_XMIT_POLICY_LAYER23");
> +			break;
> +		case BALANCE_XMIT_POLICY_LAYER34:
> +			fprintf(f, "BALANCE_XMIT_POLICY_LAYER34");
> +			break;
> +		}
why no "default case" ?
> +		fprintf(f, "\n");
> +	}
> +
> +	if (bonding_mode == BONDING_MODE_8023AD) {
> +		fprintf(f, "\tIEEE802.3AD Aggregator Mode: ");
> +		switch (internals->mode4.agg_selection) {
> +		case AGG_BANDWIDTH:
> +			fprintf(f, "bandwidth");
> +			break;
> +		case AGG_STABLE:
> +			fprintf(f, "stable");
> +			break;
> +		case AGG_COUNT:
> +			fprintf(f, "count");
> +			break;
> +		}
same as above.
> +		fprintf(f, "\n");
> +	}
> +
> +	if (internals->slave_count > 0) {
> +		fprintf(f, "\tSlaves (%u): [", internals->slave_count);
> +		for (i = 0; i < internals->slave_count - 1; i++)
> +			fprintf(f, "%u ", internals->slaves[i].port_id);
> +
> +		fprintf(f, "%u]\n", internals->slaves[internals->slave_count - 1].port_id);
> +	} else {
> +		fprintf(f, "\tSlaves: []\n");
> +	}
> +
> +	if (internals->active_slave_count > 0) {
> +		fprintf(f, "\tActive Slaves (%u): [", internals->active_slave_count);
> +		for (i = 0; i < internals->active_slave_count - 1; i++)
> +			fprintf(f, "%u ", internals->active_slaves[i]);
> +
> +		fprintf(f, "%u]\n", internals->active_slaves[internals->active_slave_count - 1]);
> +
> +	} else {
> +		fprintf(f, "\tActive Slaves: []\n");
> +	}
> +
> +	if (internals->user_defined_primary_port)
> +		fprintf(f, "\tUser Defined Primary: [%u]\n", internals->primary_port);
> +	if (internals->slave_count > 0)
> +		fprintf(f, "\tCurrent Primary: [%u]\n", internals->current_primary_port);
> +
> +	return 0;
> +}
> +
>   const struct eth_dev_ops default_dev_ops = {
>   	.dev_start            = bond_ethdev_start,
>   	.dev_stop             = bond_ethdev_stop,
> @@ -3355,7 +3455,8 @@ const struct eth_dev_ops default_dev_ops = {
>   	.mac_addr_set         = bond_ethdev_mac_address_set,
>   	.mac_addr_add         = bond_ethdev_mac_addr_add,
>   	.mac_addr_remove      = bond_ethdev_mac_addr_remove,
> -	.flow_ops_get         = bond_flow_ops_get
> +	.flow_ops_get         = bond_flow_ops_get,
> +	.eth_dev_priv_dump    = bond_ethdev_priv_dump
>   };
>   
>   static int
  
fengchengwen Dec. 14, 2022, 6:24 a.m. UTC | #2
Hi Min,

  V2 has been solved. Please review. Thanks.

On 2022/12/6 10:01, humin (Q) wrote:
> Hi,
> 
> 在 2022/12/5 16:10, Chengwen Feng 写道:
>> This patch implements eth_dev_priv_dump ops which could enhance the
>> debug capability.
>>
>> The dump output is similar to testpmd command
>> "show bonding config [port]".
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>   drivers/net/bonding/rte_eth_bond_pmd.c | 103 ++++++++++++++++++++++++-
>>   1 file changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
>> index b9bcebc6cb..80fb2dc462 100644
>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>> @@ -3329,6 +3329,106 @@ bond_ethdev_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
>>       rte_spinlock_unlock(&internals->lock);
>>   }
>>   +static const char *
>> +bond_mode_name(uint8_t mode)
>> +{
>> +    switch (mode) {
>> +    case BONDING_MODE_ROUND_ROBIN:
>> +        return "ROUND_ROBIN";
>> +    case BONDING_MODE_ACTIVE_BACKUP:
>> +        return "ACTIVE_BACKUP";
>> +    case BONDING_MODE_BALANCE:
>> +        return "BALANCE";
>> +    case BONDING_MODE_BROADCAST:
>> +        return "BROADCAST";
>> +    case BONDING_MODE_8023AD:
>> +        return "8023AD";
>> +    case BONDING_MODE_TLB:
>> +        return "TLB";
>> +    case BONDING_MODE_ALB:
>> +        return "ALB";
>> +    default:
>> +        return "Unknown";
>> +    }
>> +}
>> +
>> +static int
>> +bond_ethdev_priv_dump(struct rte_eth_dev *dev, FILE *f)
>> +{
>> +    struct bond_dev_private instant_priv;
>> +    const struct bond_dev_private *internals = &instant_priv;
>> +    int bonding_mode;
>> +    int i;
>> +
>> +    /* Obtain a instance of dev_private to prevent data from being modified. */
>> +    memcpy(&instant_priv, dev->data->dev_private, sizeof(struct bond_dev_private));
>> +    bonding_mode = internals->mode;
>> +
>> +    fprintf(f, "  - Dev basic:\n");
>> +    fprintf(f, "\tBonding mode: %s(%d)\n", bond_mode_name(bonding_mode), bonding_mode);
> data type of“bonding mode”does not match "mode".
>> +
>> +    if (bonding_mode == BONDING_MODE_BALANCE ||
>> +        bonding_mode == BONDING_MODE_8023AD) {
>> +        fprintf(f, "\tBalance Xmit Policy: ");
>> +        switch (internals->balance_xmit_policy) {
>> +        case BALANCE_XMIT_POLICY_LAYER2:
>> +            fprintf(f, "BALANCE_XMIT_POLICY_LAYER2");
>> +            break;
>> +        case BALANCE_XMIT_POLICY_LAYER23:
>> +            fprintf(f, "BALANCE_XMIT_POLICY_LAYER23");
>> +            break;
>> +        case BALANCE_XMIT_POLICY_LAYER34:
>> +            fprintf(f, "BALANCE_XMIT_POLICY_LAYER34");
>> +            break;
>> +        }
> why no "default case" ?
>> +        fprintf(f, "\n");
>> +    }
>> +
>> +    if (bonding_mode == BONDING_MODE_8023AD) {
>> +        fprintf(f, "\tIEEE802.3AD Aggregator Mode: ");
>> +        switch (internals->mode4.agg_selection) {
>> +        case AGG_BANDWIDTH:
>> +            fprintf(f, "bandwidth");
>> +            break;
>> +        case AGG_STABLE:
>> +            fprintf(f, "stable");
>> +            break;
>> +        case AGG_COUNT:
>> +            fprintf(f, "count");
>> +            break;
>> +        }
> same as above.
>> +        fprintf(f, "\n");
>> +    }
>> +
>> +    if (internals->slave_count > 0) {
>> +        fprintf(f, "\tSlaves (%u): [", internals->slave_count);
>> +        for (i = 0; i < internals->slave_count - 1; i++)
>> +            fprintf(f, "%u ", internals->slaves[i].port_id);
>> +
>> +        fprintf(f, "%u]\n", internals->slaves[internals->slave_count - 1].port_id);
>> +    } else {
>> +        fprintf(f, "\tSlaves: []\n");
>> +    }
>> +
>> +    if (internals->active_slave_count > 0) {
>> +        fprintf(f, "\tActive Slaves (%u): [", internals->active_slave_count);
>> +        for (i = 0; i < internals->active_slave_count - 1; i++)
>> +            fprintf(f, "%u ", internals->active_slaves[i]);
>> +
>> +        fprintf(f, "%u]\n", internals->active_slaves[internals->active_slave_count - 1]);
>> +
>> +    } else {
>> +        fprintf(f, "\tActive Slaves: []\n");
>> +    }
>> +
>> +    if (internals->user_defined_primary_port)
>> +        fprintf(f, "\tUser Defined Primary: [%u]\n", internals->primary_port);
>> +    if (internals->slave_count > 0)
>> +        fprintf(f, "\tCurrent Primary: [%u]\n", internals->current_primary_port);
>> +
>> +    return 0;
>> +}
>> +
>>   const struct eth_dev_ops default_dev_ops = {
>>       .dev_start            = bond_ethdev_start,
>>       .dev_stop             = bond_ethdev_stop,
>> @@ -3355,7 +3455,8 @@ const struct eth_dev_ops default_dev_ops = {
>>       .mac_addr_set         = bond_ethdev_mac_address_set,
>>       .mac_addr_add         = bond_ethdev_mac_addr_add,
>>       .mac_addr_remove      = bond_ethdev_mac_addr_remove,
>> -    .flow_ops_get         = bond_flow_ops_get
>> +    .flow_ops_get         = bond_flow_ops_get,
>> +    .eth_dev_priv_dump    = bond_ethdev_priv_dump
>>   };
>>     static int
> .
  

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b9bcebc6cb..80fb2dc462 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3329,6 +3329,106 @@  bond_ethdev_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
 	rte_spinlock_unlock(&internals->lock);
 }
 
+static const char *
+bond_mode_name(uint8_t mode)
+{
+	switch (mode) {
+	case BONDING_MODE_ROUND_ROBIN:
+		return "ROUND_ROBIN";
+	case BONDING_MODE_ACTIVE_BACKUP:
+		return "ACTIVE_BACKUP";
+	case BONDING_MODE_BALANCE:
+		return "BALANCE";
+	case BONDING_MODE_BROADCAST:
+		return "BROADCAST";
+	case BONDING_MODE_8023AD:
+		return "8023AD";
+	case BONDING_MODE_TLB:
+		return "TLB";
+	case BONDING_MODE_ALB:
+		return "ALB";
+	default:
+		return "Unknown";
+	}
+}
+
+static int
+bond_ethdev_priv_dump(struct rte_eth_dev *dev, FILE *f)
+{
+	struct bond_dev_private instant_priv;
+	const struct bond_dev_private *internals = &instant_priv;
+	int bonding_mode;
+	int i;
+
+	/* Obtain a instance of dev_private to prevent data from being modified. */
+	memcpy(&instant_priv, dev->data->dev_private, sizeof(struct bond_dev_private));
+	bonding_mode = internals->mode;
+
+	fprintf(f, "  - Dev basic:\n");
+	fprintf(f, "\tBonding mode: %s(%d)\n", bond_mode_name(bonding_mode), bonding_mode);
+
+	if (bonding_mode == BONDING_MODE_BALANCE ||
+		bonding_mode == BONDING_MODE_8023AD) {
+		fprintf(f, "\tBalance Xmit Policy: ");
+		switch (internals->balance_xmit_policy) {
+		case BALANCE_XMIT_POLICY_LAYER2:
+			fprintf(f, "BALANCE_XMIT_POLICY_LAYER2");
+			break;
+		case BALANCE_XMIT_POLICY_LAYER23:
+			fprintf(f, "BALANCE_XMIT_POLICY_LAYER23");
+			break;
+		case BALANCE_XMIT_POLICY_LAYER34:
+			fprintf(f, "BALANCE_XMIT_POLICY_LAYER34");
+			break;
+		}
+		fprintf(f, "\n");
+	}
+
+	if (bonding_mode == BONDING_MODE_8023AD) {
+		fprintf(f, "\tIEEE802.3AD Aggregator Mode: ");
+		switch (internals->mode4.agg_selection) {
+		case AGG_BANDWIDTH:
+			fprintf(f, "bandwidth");
+			break;
+		case AGG_STABLE:
+			fprintf(f, "stable");
+			break;
+		case AGG_COUNT:
+			fprintf(f, "count");
+			break;
+		}
+		fprintf(f, "\n");
+	}
+
+	if (internals->slave_count > 0) {
+		fprintf(f, "\tSlaves (%u): [", internals->slave_count);
+		for (i = 0; i < internals->slave_count - 1; i++)
+			fprintf(f, "%u ", internals->slaves[i].port_id);
+
+		fprintf(f, "%u]\n", internals->slaves[internals->slave_count - 1].port_id);
+	} else {
+		fprintf(f, "\tSlaves: []\n");
+	}
+
+	if (internals->active_slave_count > 0) {
+		fprintf(f, "\tActive Slaves (%u): [", internals->active_slave_count);
+		for (i = 0; i < internals->active_slave_count - 1; i++)
+			fprintf(f, "%u ", internals->active_slaves[i]);
+
+		fprintf(f, "%u]\n", internals->active_slaves[internals->active_slave_count - 1]);
+
+	} else {
+		fprintf(f, "\tActive Slaves: []\n");
+	}
+
+	if (internals->user_defined_primary_port)
+		fprintf(f, "\tUser Defined Primary: [%u]\n", internals->primary_port);
+	if (internals->slave_count > 0)
+		fprintf(f, "\tCurrent Primary: [%u]\n", internals->current_primary_port);
+
+	return 0;
+}
+
 const struct eth_dev_ops default_dev_ops = {
 	.dev_start            = bond_ethdev_start,
 	.dev_stop             = bond_ethdev_stop,
@@ -3355,7 +3455,8 @@  const struct eth_dev_ops default_dev_ops = {
 	.mac_addr_set         = bond_ethdev_mac_address_set,
 	.mac_addr_add         = bond_ethdev_mac_addr_add,
 	.mac_addr_remove      = bond_ethdev_mac_addr_remove,
-	.flow_ops_get         = bond_flow_ops_get
+	.flow_ops_get         = bond_flow_ops_get,
+	.eth_dev_priv_dump    = bond_ethdev_priv_dump
 };
 
 static int