[dpdk-dev,v3,10/10] app/testpmd: fix on the flight VLAN configuration
Checks
Commit Message
On ethdev there is an API to configure VLAN offloads after the port
was started and without reconfiguration of the port or queues.
In the current design of the application, when the Rx offloads are
changed (through "port config all" CLI command) the port configuration
is overwritten, therefore the configuration made for the VLAN is lost.
This patch is to address the issue by a configuration of each port Rx
offloads separately instead of using the global Rx config.
Fixes: 6dbb2b336586 ("app/testpmd: convert to new Ethdev Rx offloads API")
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
app/test-pmd/cmdline.c | 217 +++++++++++++++++++++++---------------------
app/test-pmd/config.c | 27 ++++--
app/test-pmd/testpmd.c | 2 +-
3 files changed, 135 insertions(+), 111 deletions(-)
Comments
Hi Shahaf,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Tuesday, December 26, 2017 5:44 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 10/10] app/testpmd: fix on the flight VLAN
> configuration
>
> On ethdev there is an API to configure VLAN offloads after the port was
> started and without reconfiguration of the port or queues.
>
> In the current design of the application, when the Rx offloads are changed
> (through "port config all" CLI command) the port configuration is overwritten,
> therefore the configuration made for the VLAN is lost.
>
> This patch is to address the issue by a configuration of each port Rx offloads
> separately instead of using the global Rx config.
>
> Fixes: 6dbb2b336586 ("app/testpmd: convert to new Ethdev Rx offloads API")
This patch is not applied yet. The number 6dbb2b336586 could be meaningless. If the previous patch need to be fixed, may I suggest just merge this one to that? Or this patch fixes some older issues actually?
Tuesday, January 9, 2018 10:06 AM, Lu, Wenzhuo:
> > Subject: [dpdk-dev] [PATCH v3 10/10] app/testpmd: fix on the flight
> > VLAN configuration
> >
> > On ethdev there is an API to configure VLAN offloads after the port
> > was started and without reconfiguration of the port or queues.
> >
> > In the current design of the application, when the Rx offloads are
> > changed (through "port config all" CLI command) the port configuration
> > is overwritten, therefore the configuration made for the VLAN is lost.
> >
> > This patch is to address the issue by a configuration of each port Rx
> > offloads separately instead of using the global Rx config.
> >
> > Fixes: 6dbb2b336586 ("app/testpmd: convert to new Ethdev Rx offloads
> > API")
> This patch is not applied yet. The number 6dbb2b336586 could be
> meaningless. If the previous patch need to be fixed, may I suggest just
> merge this one to that? Or this patch fixes some older issues actually?
Well, this was done per Ferruh's request to better split the patches.
I can merge it with the Rx one, agreed?
Hi Shahaf,
> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Tuesday, January 9, 2018 6:04 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 10/10] app/testpmd: fix on the flight
> VLAN configuration
>
> Tuesday, January 9, 2018 10:06 AM, Lu, Wenzhuo:
> > > Subject: [dpdk-dev] [PATCH v3 10/10] app/testpmd: fix on the flight
> > > VLAN configuration
> > >
> > > On ethdev there is an API to configure VLAN offloads after the port
> > > was started and without reconfiguration of the port or queues.
> > >
> > > In the current design of the application, when the Rx offloads are
> > > changed (through "port config all" CLI command) the port
> > > configuration is overwritten, therefore the configuration made for the
> VLAN is lost.
> > >
> > > This patch is to address the issue by a configuration of each port
> > > Rx offloads separately instead of using the global Rx config.
> > >
> > > Fixes: 6dbb2b336586 ("app/testpmd: convert to new Ethdev Rx offloads
> > > API")
> > This patch is not applied yet. The number 6dbb2b336586 could be
> > meaningless. If the previous patch need to be fixed, may I suggest
> > just merge this one to that? Or this patch fixes some older issues actually?
>
> Well, this was done per Ferruh's request to better split the patches.
>
> I can merge it with the Rx one, agreed?
I think this patch fixes the bug in the old code. This bug is not introduced by "app/testpmd: convert to new Ethdev Rx offloads API". You set this fixes tag only because the code is changed so much by "app/testpmd: convert to new Ethdev Rx offloads API". It's helpful to create an individual patch. To my opinion, just remove the fixes tag is enough.
@@ -1577,34 +1577,38 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
__attribute__((unused)) void *data)
{
struct cmd_config_max_pkt_len_result *res = parsed_result;
- uint64_t rx_offloads = rx_mode.offloads;
+ portid_t pid;
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
return;
}
- if (!strcmp(res->name, "max-pkt-len")) {
- if (res->value < ETHER_MIN_LEN) {
- printf("max-pkt-len can not be less than %d\n",
- ETHER_MIN_LEN);
+ RTE_ETH_FOREACH_DEV(pid) {
+ struct rte_port *port = &ports[pid];
+ uint64_t rx_offloads = port->dev_conf.rxmode.offloads;
+
+ if (!strcmp(res->name, "max-pkt-len")) {
+ if (res->value < ETHER_MIN_LEN) {
+ printf("max-pkt-len can not be less than %d\n",
+ ETHER_MIN_LEN);
+ return;
+ }
+ if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)
+ return;
+
+ port->dev_conf.rxmode.max_rx_pkt_len = res->value;
+ if (res->value > ETHER_MAX_LEN)
+ rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+ else
+ rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+ port->dev_conf.rxmode.offloads = rx_offloads;
+ } else {
+ printf("Unknown parameter\n");
return;
}
- if (res->value == rx_mode.max_rx_pkt_len)
- return;
-
- rx_mode.max_rx_pkt_len = res->value;
- if (res->value > ETHER_MAX_LEN)
- rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
- else
- rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
- } else {
- printf("Unknown parameter\n");
- return;
}
- rx_mode.offloads = rx_offloads;
-
init_port_config();
cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
@@ -1706,103 +1710,108 @@ cmd_config_rx_mode_flag_parsed(void *parsed_result,
__attribute__((unused)) void *data)
{
struct cmd_config_rx_mode_flag *res = parsed_result;
- uint64_t rx_offloads = rx_mode.offloads;
+ portid_t pid;
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
return;
}
- if (!strcmp(res->name, "crc-strip")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "scatter")) {
- if (!strcmp(res->value, "on")) {
- rx_offloads |= DEV_RX_OFFLOAD_SCATTER;
- } else if (!strcmp(res->value, "off")) {
- rx_offloads &= ~DEV_RX_OFFLOAD_SCATTER;
+ RTE_ETH_FOREACH_DEV(pid) {
+ struct rte_port *port;
+ uint64_t rx_offloads;
+
+ port = &ports[pid];
+ rx_offloads = port->dev_conf.rxmode.offloads;
+ if (!strcmp(res->name, "crc-strip")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_CRC_STRIP;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "scatter")) {
+ if (!strcmp(res->value, "on")) {
+ rx_offloads |= DEV_RX_OFFLOAD_SCATTER;
+ } else if (!strcmp(res->value, "off")) {
+ rx_offloads &= ~DEV_RX_OFFLOAD_SCATTER;
+ } else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "rx-cksum")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_CHECKSUM;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_CHECKSUM;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "rx-timestamp")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_TIMESTAMP;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_TIMESTAMP;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "hw-vlan")) {
+ if (!strcmp(res->value, "on")) {
+ rx_offloads |= (DEV_RX_OFFLOAD_VLAN_FILTER |
+ DEV_RX_OFFLOAD_VLAN_STRIP);
+ } else if (!strcmp(res->value, "off")) {
+ rx_offloads &= ~(DEV_RX_OFFLOAD_VLAN_FILTER |
+ DEV_RX_OFFLOAD_VLAN_STRIP);
+ } else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "hw-vlan-filter")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_VLAN_FILTER;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_FILTER;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "hw-vlan-strip")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "hw-vlan-extend")) {
+ if (!strcmp(res->value, "on"))
+ rx_offloads |= DEV_RX_OFFLOAD_VLAN_EXTEND;
+ else if (!strcmp(res->value, "off"))
+ rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_EXTEND;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
+ } else if (!strcmp(res->name, "drop-en")) {
+ if (!strcmp(res->value, "on"))
+ rx_drop_en = 1;
+ else if (!strcmp(res->value, "off"))
+ rx_drop_en = 0;
+ else {
+ printf("Unknown parameter\n");
+ return;
+ }
} else {
printf("Unknown parameter\n");
return;
}
- } else if (!strcmp(res->name, "rx-cksum")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_CHECKSUM;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_CHECKSUM;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "rx-timestamp")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_TIMESTAMP;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_TIMESTAMP;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "hw-vlan")) {
- if (!strcmp(res->value, "on")) {
- rx_offloads |= (DEV_RX_OFFLOAD_VLAN_FILTER |
- DEV_RX_OFFLOAD_VLAN_STRIP);
- }
- else if (!strcmp(res->value, "off")) {
- rx_offloads &= ~(DEV_RX_OFFLOAD_VLAN_FILTER |
- DEV_RX_OFFLOAD_VLAN_STRIP);
- }
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "hw-vlan-filter")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_VLAN_FILTER;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_FILTER;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "hw-vlan-strip")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "hw-vlan-extend")) {
- if (!strcmp(res->value, "on"))
- rx_offloads |= DEV_RX_OFFLOAD_VLAN_EXTEND;
- else if (!strcmp(res->value, "off"))
- rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_EXTEND;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else if (!strcmp(res->name, "drop-en")) {
- if (!strcmp(res->value, "on"))
- rx_drop_en = 1;
- else if (!strcmp(res->value, "off"))
- rx_drop_en = 0;
- else {
- printf("Unknown parameter\n");
- return;
- }
- } else {
- printf("Unknown parameter\n");
- return;
+ port->dev_conf.rxmode.offloads = rx_offloads;
}
- rx_mode.offloads = rx_offloads;
init_port_config();
@@ -2665,21 +2665,26 @@ vlan_extend_set(portid_t port_id, int on)
{
int diag;
int vlan_offload;
+ uint64_t port_rx_offloads = ports[port_id].dev_conf.rxmode.offloads;
if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
- if (on)
+ if (on) {
vlan_offload |= ETH_VLAN_EXTEND_OFFLOAD;
- else
+ port_rx_offloads |= DEV_RX_OFFLOAD_VLAN_EXTEND;
+ } else {
vlan_offload &= ~ETH_VLAN_EXTEND_OFFLOAD;
+ port_rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_EXTEND;
+ }
diag = rte_eth_dev_set_vlan_offload(port_id, vlan_offload);
if (diag < 0)
printf("rx_vlan_extend_set(port_pi=%d, on=%d) failed "
"diag=%d\n", port_id, on, diag);
+ ports[port_id].dev_conf.rxmode.offloads = port_rx_offloads;
}
void
@@ -2687,21 +2692,26 @@ rx_vlan_strip_set(portid_t port_id, int on)
{
int diag;
int vlan_offload;
+ uint64_t port_rx_offloads = ports[port_id].dev_conf.rxmode.offloads;
if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
- if (on)
+ if (on) {
vlan_offload |= ETH_VLAN_STRIP_OFFLOAD;
- else
+ port_rx_offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
+ } else {
vlan_offload &= ~ETH_VLAN_STRIP_OFFLOAD;
+ port_rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
+ }
diag = rte_eth_dev_set_vlan_offload(port_id, vlan_offload);
if (diag < 0)
printf("rx_vlan_strip_set(port_pi=%d, on=%d) failed "
"diag=%d\n", port_id, on, diag);
+ ports[port_id].dev_conf.rxmode.offloads = port_rx_offloads;
}
void
@@ -2723,21 +2733,26 @@ rx_vlan_filter_set(portid_t port_id, int on)
{
int diag;
int vlan_offload;
+ uint64_t port_rx_offloads = ports[port_id].dev_conf.rxmode.offloads;
if (port_id_is_invalid(port_id, ENABLED_WARN))
return;
vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
- if (on)
+ if (on) {
vlan_offload |= ETH_VLAN_FILTER_OFFLOAD;
- else
+ port_rx_offloads |= DEV_RX_OFFLOAD_VLAN_FILTER;
+ } else {
vlan_offload &= ~ETH_VLAN_FILTER_OFFLOAD;
+ port_rx_offloads &= ~DEV_RX_OFFLOAD_VLAN_FILTER;
+ }
diag = rte_eth_dev_set_vlan_offload(port_id, vlan_offload);
if (diag < 0)
printf("rx_vlan_filter_set(port_pi=%d, on=%d) failed "
"diag=%d\n", port_id, on, diag);
+ ports[port_id].dev_conf.rxmode.offloads = port_rx_offloads;
}
int
@@ -603,6 +603,7 @@ init_config(void)
port = &ports[pid];
/* Apply default Tx configuration for all ports */
port->dev_conf.txmode = tx_mode;
+ port->dev_conf.rxmode = rx_mode;
rte_eth_dev_info_get(pid, &port->dev_info);
if (numa_support) {
@@ -2089,7 +2090,6 @@ init_port_config(void)
RTE_ETH_FOREACH_DEV(pid) {
port = &ports[pid];
- port->dev_conf.rxmode = rx_mode;
port->dev_conf.fdir_conf = fdir_conf;
if (nb_rxq > 1) {
port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;