* [PATCH 0/2] examples/kni: update kni example application @ 2017-10-16 11:45 Tomasz Duszynski 2017-10-16 11:45 ` [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it Tomasz Duszynski 2017-10-16 11:45 ` [PATCH 2/2] examples/kni: stop lcores while doing kni ops Tomasz Duszynski 0 siblings, 2 replies; 7+ messages in thread From: Tomasz Duszynski @ 2017-10-16 11:45 UTC (permalink / raw) To: dev; +Cc: Tomasz Duszynski This patch series introduces some changes to the kni example application so that non-pci devices and those that can cannot do rx/tx while being stopped can use it. Tomasz Duszynski (2): examples/kni: check if pci_dev isn't NULL before using it examples/kni: stop lcores while doing kni ops examples/kni/main.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it 2017-10-16 11:45 [PATCH 0/2] examples/kni: update kni example application Tomasz Duszynski @ 2017-10-16 11:45 ` Tomasz Duszynski 2017-10-21 0:26 ` Ferruh Yigit 2017-10-16 11:45 ` [PATCH 2/2] examples/kni: stop lcores while doing kni ops Tomasz Duszynski 1 sibling, 1 reply; 7+ messages in thread From: Tomasz Duszynski @ 2017-10-16 11:45 UTC (permalink / raw) To: dev; +Cc: Tomasz Duszynski Since virtual devices, i.e mrvl net pmd, do not touch pci_dev dereferencing it will cause segmentation fault as by default it's set to NULL in rte_eth_dev_info_get(). Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> --- examples/kni/main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/kni/main.c b/examples/kni/main.c index 6d9e4a6..cb48fb5 100644 --- a/examples/kni/main.c +++ b/examples/kni/main.c @@ -805,8 +805,11 @@ kni_alloc(uint16_t port_id) memset(&dev_info, 0, sizeof(dev_info)); rte_eth_dev_info_get(port_id, &dev_info); - conf.addr = dev_info.pci_dev->addr; - conf.id = dev_info.pci_dev->id; + + if (dev_info.pci_dev) { + conf.addr = dev_info.pci_dev->addr; + conf.id = dev_info.pci_dev->id; + } memset(&ops, 0, sizeof(ops)); ops.port_id = port_id; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it 2017-10-16 11:45 ` [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it Tomasz Duszynski @ 2017-10-21 0:26 ` Ferruh Yigit 2017-10-24 22:05 ` Thomas Monjalon 0 siblings, 1 reply; 7+ messages in thread From: Ferruh Yigit @ 2017-10-21 0:26 UTC (permalink / raw) To: Tomasz Duszynski, dev On 10/16/2017 4:45 AM, Tomasz Duszynski wrote: > Since virtual devices, i.e mrvl net pmd, do not touch pci_dev > dereferencing it will cause segmentation fault as by default > it's set to NULL in rte_eth_dev_info_get(). > > Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it 2017-10-21 0:26 ` Ferruh Yigit @ 2017-10-24 22:05 ` Thomas Monjalon 0 siblings, 0 replies; 7+ messages in thread From: Thomas Monjalon @ 2017-10-24 22:05 UTC (permalink / raw) To: Tomasz Duszynski; +Cc: dev, Ferruh Yigit 21/10/2017 02:26, Ferruh Yigit: > On 10/16/2017 4:45 AM, Tomasz Duszynski wrote: > > Since virtual devices, i.e mrvl net pmd, do not touch pci_dev > > dereferencing it will cause segmentation fault as by default > > it's set to NULL in rte_eth_dev_info_get(). > > > > Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> > > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> Fixed title: examples/kni: check PCI info not NULL before reading Applied, thanks The patch 2/2 is left alone as it may be unneeded. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] examples/kni: stop lcores while doing kni ops 2017-10-16 11:45 [PATCH 0/2] examples/kni: update kni example application Tomasz Duszynski 2017-10-16 11:45 ` [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it Tomasz Duszynski @ 2017-10-16 11:45 ` Tomasz Duszynski 2017-10-21 0:42 ` Ferruh Yigit 1 sibling, 1 reply; 7+ messages in thread From: Tomasz Duszynski @ 2017-10-16 11:45 UTC (permalink / raw) To: dev; +Cc: Tomasz Duszynski Since the transmit and receive functions should not be invoked when the device is stopped, stop lcores during kni ops and restart them after device is started once again. Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> --- examples/kni/main.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/examples/kni/main.c b/examples/kni/main.c index cb48fb5..5c50448 100644 --- a/examples/kni/main.c +++ b/examples/kni/main.c @@ -166,6 +166,23 @@ static int kni_change_mtu(uint16_t port_id, unsigned int new_mtu); static int kni_config_network_interface(uint16_t port_id, uint8_t if_up); static rte_atomic32_t kni_stop = RTE_ATOMIC32_INIT(0); +static rte_atomic32_t kni_restart = RTE_ATOMIC32_INIT(0); + +static void +kni_stop_lcores(void) +{ + unsigned int i; + + rte_atomic32_inc(&kni_restart); + rte_atomic32_inc(&kni_stop); + + RTE_LCORE_FOREACH(i) { + if (i == rte_lcore_id()) + continue; + + rte_eal_wait_lcore(i); + } +} /* Print out statistics on packets handled */ static void @@ -712,6 +729,7 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu) RTE_LOG(INFO, APP, "Change MTU of port %d to %u\n", port_id, new_mtu); + kni_stop_lcores(); /* Stop specific port */ rte_eth_dev_stop(port_id); @@ -755,6 +773,8 @@ kni_config_network_interface(uint16_t port_id, uint8_t if_up) RTE_LOG(INFO, APP, "Configure network interface of %d %s\n", port_id, if_up ? "up" : "down"); + kni_stop_lcores(); + if (if_up != 0) { /* Configure network interface up */ rte_eth_dev_stop(port_id); ret = rte_eth_dev_start(port_id); @@ -911,6 +931,7 @@ main(int argc, char** argv) } check_all_ports_link_status(nb_sys_ports, ports_mask); +restart: /* Launch per-lcore function on every lcore */ rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); RTE_LCORE_FOREACH_SLAVE(i) { @@ -918,6 +939,13 @@ main(int argc, char** argv) return -1; } + if (rte_atomic32_read(&kni_restart)) { + rte_atomic32_dec(&kni_stop); + rte_atomic32_dec(&kni_restart); + + goto restart; + } + /* Release resources */ for (port = 0; port < nb_sys_ports; port++) { if (!(ports_mask & (1 << port))) -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] examples/kni: stop lcores while doing kni ops 2017-10-16 11:45 ` [PATCH 2/2] examples/kni: stop lcores while doing kni ops Tomasz Duszynski @ 2017-10-21 0:42 ` Ferruh Yigit 2017-10-25 9:28 ` Tomasz Duszynski 0 siblings, 1 reply; 7+ messages in thread From: Ferruh Yigit @ 2017-10-21 0:42 UTC (permalink / raw) To: Tomasz Duszynski, dev On 10/16/2017 4:45 AM, Tomasz Duszynski wrote: > Since the transmit and receive functions should not be invoked when > the device is stopped, stop lcores during kni ops and restart them > after device is started once again. Hi Tomasz, Are you observing any error or unexpected behavior because of rx/tx functions? I am not sure about the patch, please check below logs, and trying to understand scope of the patch, can you please give more details what happens if this patch is missing? > > Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> > --- > examples/kni/main.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/examples/kni/main.c b/examples/kni/main.c > index cb48fb5..5c50448 100644 > --- a/examples/kni/main.c > +++ b/examples/kni/main.c > @@ -166,6 +166,23 @@ static int kni_change_mtu(uint16_t port_id, unsigned int new_mtu); > static int kni_config_network_interface(uint16_t port_id, uint8_t if_up); > > static rte_atomic32_t kni_stop = RTE_ATOMIC32_INIT(0); > +static rte_atomic32_t kni_restart = RTE_ATOMIC32_INIT(0); > + > +static void > +kni_stop_lcores(void) > +{ > + unsigned int i; > + > + rte_atomic32_inc(&kni_restart); > + rte_atomic32_inc(&kni_stop); > + > + RTE_LCORE_FOREACH(i) { > + if (i == rte_lcore_id()) > + continue; This function called by port Rx core [1], and since the thread can't wait itself to finish, specially if nb_kni > 1, the Rx core still can do some work even after exit from this function. > + > + rte_eal_wait_lcore(i); The API documentation says: "To be executed on the MASTER lcore only." Not sure what happens when called from slave core, as we did here. > + } > +} > > /* Print out statistics on packets handled */ > static void > @@ -712,6 +729,7 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu) > > RTE_LOG(INFO, APP, "Change MTU of port %d to %u\n", port_id, new_mtu); > > + kni_stop_lcores(); > /* Stop specific port */ > rte_eth_dev_stop(port_id); > > @@ -755,6 +773,8 @@ kni_config_network_interface(uint16_t port_id, uint8_t if_up) > RTE_LOG(INFO, APP, "Configure network interface of %d %s\n", > port_id, if_up ? "up" : "down"); > > + kni_stop_lcores(); > + > if (if_up != 0) { /* Configure network interface up */ > rte_eth_dev_stop(port_id); > ret = rte_eth_dev_start(port_id); > @@ -911,6 +931,7 @@ main(int argc, char** argv) > } > check_all_ports_link_status(nb_sys_ports, ports_mask); > > +restart: > /* Launch per-lcore function on every lcore */ > rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); > RTE_LCORE_FOREACH_SLAVE(i) { > @@ -918,6 +939,13 @@ main(int argc, char** argv) > return -1; > } > > + if (rte_atomic32_read(&kni_restart)) { > + rte_atomic32_dec(&kni_stop); > + rte_atomic32_dec(&kni_restart); kni_stop_lcores() called per port, so it is possible that kni_stop and kni_restart increased parallel, many times. But this decrement is per application, so they will be decremented sequentially, casing app stop - start unnecessarily. > + > + goto restart; This will cause assigning tasks to cores again, and will produce all related logs again, if you enable debug logs you will see it [2]. I believe confusing to have those logs every time mtu updated etc... > + } > + > /* Release resources */ > for (port = 0; port < nb_sys_ports; port++) { > if (!(ports_mask & (1 << port))) > -- > 2.7.4 > [1] main_loop kni_ingress rte_kni_handle_request kni_change_mtu || kni_config_network_interface kni_stop_lcores [2] APP: Change MTU of port 0 to 1402 PMD: ixgbe_set_rx_function(): Vector rx enabled, please make sure RX burst size no less than 4 (port=0). PMD: ixgbe_dev_link_status_print(): Port 0: Link Down PMD: ixgbe_dev_link_status_print(): PCI Address: 0000:08:00.1 APP: Lcore 1 is reading from port 0 APP: Lcore 2 is writing to port 0 APP: Lcore 3 is reading from port 1 APP: Lcore 4 is writing to port 1 APP: Lcore 5 has nothing to do APP: Lcore 6 has nothing to do APP: Lcore 7 has nothing to do APP: Lcore 8 has nothing to do APP: Lcore 9 has nothing to do APP: Lcore 10 has nothing to do APP: Lcore 11 has nothing to do APP: Lcore 12 has nothing to do APP: Lcore 13 has nothing to do APP: Lcore 14 has nothing to do APP: Lcore 15 has nothing to do APP: Lcore 16 has nothing to do APP: Lcore 17 has nothing to do APP: Lcore 18 has nothing to do APP: Lcore 19 has nothing to do APP: Lcore 20 has nothing to do APP: Lcore 21 has nothing to do APP: Lcore 22 has nothing to do APP: Lcore 23 has nothing to do APP: Lcore 24 has nothing to do APP: Lcore 25 has nothing to do APP: Lcore 26 has nothing to do APP: Lcore 27 has nothing to do APP: Lcore 28 has nothing to do APP: Lcore 29 has nothing to do APP: Lcore 30 has nothing to do APP: Lcore 31 has nothing to do APP: Lcore 0 has nothing to do ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] examples/kni: stop lcores while doing kni ops 2017-10-21 0:42 ` Ferruh Yigit @ 2017-10-25 9:28 ` Tomasz Duszynski 0 siblings, 0 replies; 7+ messages in thread From: Tomasz Duszynski @ 2017-10-25 9:28 UTC (permalink / raw) To: Ferruh Yigit; +Cc: Tomasz Duszynski, dev Hi Ferruh, On Fri, Oct 20, 2017 at 05:42:13PM -0700, Ferruh Yigit wrote: > On 10/16/2017 4:45 AM, Tomasz Duszynski wrote: > > Since the transmit and receive functions should not be invoked when > > the device is stopped, stop lcores during kni ops and restart them > > after device is started once again. > > Hi Tomasz, > > Are you observing any error or unexpected behavior because of rx/tx functions? I > am not sure about the patch, please check below logs, and trying to understand > scope of the patch, can you please give more details what happens if this patch > is missing? Right, calling rx/tx functions after device was stopped will break things. For instace, putting interface up will call rte_eth_dev_stop() which frees port resources. If that happens (and usually happens) during rx/tx driver would break, as resources used in library functions calls are bogus at that point. So we end up with SIGSEGV. According to dpdk documentation rx/tx functions cannot be called before dev_start(). Thus I think cores that do rx/tx should be stopped just before rte_eth_dev_stop() is called. That could be fixed inside pmd but would require locks in rx/tx path which is rather a no-go. > > > > > Signed-off-by: Tomasz Duszynski <tdu@semihalf.com> > > --- > > examples/kni/main.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/examples/kni/main.c b/examples/kni/main.c > > index cb48fb5..5c50448 100644 > > --- a/examples/kni/main.c > > +++ b/examples/kni/main.c > > @@ -166,6 +166,23 @@ static int kni_change_mtu(uint16_t port_id, unsigned int new_mtu); > > static int kni_config_network_interface(uint16_t port_id, uint8_t if_up); > > > > static rte_atomic32_t kni_stop = RTE_ATOMIC32_INIT(0); > > +static rte_atomic32_t kni_restart = RTE_ATOMIC32_INIT(0); > > + > > +static void > > +kni_stop_lcores(void) > > +{ > > + unsigned int i; > > + > > + rte_atomic32_inc(&kni_restart); > > + rte_atomic32_inc(&kni_stop); > > + > > + RTE_LCORE_FOREACH(i) { > > + if (i == rte_lcore_id()) > > + continue; > > This function called by port Rx core [1], and since the thread can't wait itself > to finish, specially if nb_kni > 1, the Rx core still can do some work even > after exit from this function. Ack. > > > + > > + rte_eal_wait_lcore(i); > > The API documentation says: "To be executed on the MASTER lcore only." > Not sure what happens when called from slave core, as we did here. OK. > > > + } > > +} > > > > /* Print out statistics on packets handled */ > > static void > > @@ -712,6 +729,7 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu) > > > > RTE_LOG(INFO, APP, "Change MTU of port %d to %u\n", port_id, new_mtu); > > > > + kni_stop_lcores(); > > /* Stop specific port */ > > rte_eth_dev_stop(port_id); > > > > @@ -755,6 +773,8 @@ kni_config_network_interface(uint16_t port_id, uint8_t if_up) > > RTE_LOG(INFO, APP, "Configure network interface of %d %s\n", > > port_id, if_up ? "up" : "down"); > > > > + kni_stop_lcores(); > > + > > if (if_up != 0) { /* Configure network interface up */ > > rte_eth_dev_stop(port_id); > > ret = rte_eth_dev_start(port_id); > > @@ -911,6 +931,7 @@ main(int argc, char** argv) > > } > > check_all_ports_link_status(nb_sys_ports, ports_mask); > > > > +restart: > > /* Launch per-lcore function on every lcore */ > > rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); > > RTE_LCORE_FOREACH_SLAVE(i) { > > @@ -918,6 +939,13 @@ main(int argc, char** argv) > > return -1; > > } > > > > + if (rte_atomic32_read(&kni_restart)) { > > + rte_atomic32_dec(&kni_stop); > > + rte_atomic32_dec(&kni_restart); > > kni_stop_lcores() called per port, so it is possible that kni_stop and > kni_restart increased parallel, many times. But this decrement is per > application, so they will be decremented sequentially, casing app stop - start > unnecessarily. Right. > > > + > > + goto restart; > > This will cause assigning tasks to cores again, and will produce all related > logs again, if you enable debug logs you will see it [2]. I believe confusing to > have those logs every time mtu updated etc... Right. > > > + } > > + > > /* Release resources */ > > for (port = 0; port < nb_sys_ports; port++) { > > if (!(ports_mask & (1 << port))) > > -- > > 2.7.4 > > > > [1] > main_loop > kni_ingress > rte_kni_handle_request > kni_change_mtu > || > kni_config_network_interface > kni_stop_lcores > > > [2] > APP: Change MTU of port 0 to 1402 > PMD: ixgbe_set_rx_function(): Vector rx enabled, please make sure RX burst size > no less than 4 (port=0). > PMD: ixgbe_dev_link_status_print(): Port 0: Link Down > PMD: ixgbe_dev_link_status_print(): PCI Address: 0000:08:00.1 > APP: Lcore 1 is reading from port 0 > APP: Lcore 2 is writing to port 0 > APP: Lcore 3 is reading from port 1 > APP: Lcore 4 is writing to port 1 > APP: Lcore 5 has nothing to do > APP: Lcore 6 has nothing to do > APP: Lcore 7 has nothing to do > APP: Lcore 8 has nothing to do > APP: Lcore 9 has nothing to do > APP: Lcore 10 has nothing to do > APP: Lcore 11 has nothing to do > APP: Lcore 12 has nothing to do > APP: Lcore 13 has nothing to do > APP: Lcore 14 has nothing to do > APP: Lcore 15 has nothing to do > APP: Lcore 16 has nothing to do > APP: Lcore 17 has nothing to do > APP: Lcore 18 has nothing to do > APP: Lcore 19 has nothing to do > APP: Lcore 20 has nothing to do > APP: Lcore 21 has nothing to do > APP: Lcore 22 has nothing to do > APP: Lcore 23 has nothing to do > APP: Lcore 24 has nothing to do > APP: Lcore 25 has nothing to do > APP: Lcore 26 has nothing to do > APP: Lcore 27 has nothing to do > APP: Lcore 28 has nothing to do > APP: Lcore 29 has nothing to do > APP: Lcore 30 has nothing to do > APP: Lcore 31 has nothing to do > APP: Lcore 0 has nothing to do -- - Tomasz Duszyński ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-25 9:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-16 11:45 [PATCH 0/2] examples/kni: update kni example application Tomasz Duszynski 2017-10-16 11:45 ` [PATCH 1/2] examples/kni: check if pci_dev isn't NULL before using it Tomasz Duszynski 2017-10-21 0:26 ` Ferruh Yigit 2017-10-24 22:05 ` Thomas Monjalon 2017-10-16 11:45 ` [PATCH 2/2] examples/kni: stop lcores while doing kni ops Tomasz Duszynski 2017-10-21 0:42 ` Ferruh Yigit 2017-10-25 9:28 ` Tomasz Duszynski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.