From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Nicol=C3=A1s_Pernas_Maradei?= Subject: Re: =?utf-8?q?=5BPATCH_v2=5D_Fix_librte=5Fpmd=5Fpcap_drive?= =?utf-8?q?r_double_stop_error?= Date: Sun, 05 Oct 2014 19:37:11 +0100 Message-ID: <209a96ce6067cf0b5239b8d9143fbb2a@statler.emutex.com> References: <005307> <1412461457-29061-1-git-send-email-nico@emutex.com> Reply-To: nico-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable To: dev-VfR2kkLFssw@public.gmane.org Return-path: In-Reply-To: <1412461457-29061-1-git-send-email-nico-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi, New in Patch v2: - Fixes an issue in eth_dev_start/stop where a single interface was=20 always opened/closed even though pcap files had been selected for rx/tx=20 streams. - The link_status was not being properly updated in case of using a=20 single interface for rx/tx streams. Nico. On 2014-10-04 23:24, Nicol=C3=A1s Pernas Maradei wrote: > From: Nicol=C3=A1s Pernas Maradei >=20 > librte_pmd_pcap driver was opening the pcap/interfaces only at init=20 > time and > closing them only when the port was being stopped. This behaviour would= =20 > cause > problems (leading to segfault) if the user closed the port 2 times. The= =20 > first > time the pcap/interfaces would be normally closed but libpcap would=20 > throw an > error causing a segfault if the closed pcaps/interfaces were closed=20 > again. > This behaviour is solved by re-openning pcaps/interfaces when the port=20 > is > started (only if these weren't open already for example at init time). >=20 > Signed-off-by: Nicol=C3=A1s Pernas Maradei > --- > lib/librte_pmd_pcap/rte_eth_pcap.c | 271=20 > +++++++++++++++++++++++++++++-------- > 1 file changed, 212 insertions(+), 59 deletions(-) >=20 > diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c > b/lib/librte_pmd_pcap/rte_eth_pcap.c > index 19492e8..f12d1e7 100644 > --- a/lib/librte_pmd_pcap/rte_eth_pcap.c > +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c > @@ -67,6 +67,8 @@ struct pcap_rx_queue { > struct rte_mempool *mb_pool; > volatile unsigned long rx_pkts; > volatile unsigned long err_pkts; > + const char *name; > + const char *type; > }; >=20 > struct pcap_tx_queue { > @@ -74,27 +76,32 @@ struct pcap_tx_queue { > pcap_t *pcap; > volatile unsigned long tx_pkts; > volatile unsigned long err_pkts; > + const char *name; > + const char *type; > }; >=20 > struct rx_pcaps { > unsigned num_of_rx; > pcap_t *pcaps[RTE_PMD_RING_MAX_RX_RINGS]; > + const char *names[RTE_PMD_RING_MAX_RX_RINGS]; > + const char *types[RTE_PMD_RING_MAX_RX_RINGS]; > }; >=20 > struct tx_pcaps { > unsigned num_of_tx; > pcap_dumper_t *dumpers[RTE_PMD_RING_MAX_TX_RINGS]; > pcap_t *pcaps[RTE_PMD_RING_MAX_RX_RINGS]; > + const char *names[RTE_PMD_RING_MAX_RX_RINGS]; > + const char *types[RTE_PMD_RING_MAX_RX_RINGS]; > }; >=20 > struct pmd_internals { > + struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS]; > + struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS]; > unsigned nb_rx_queues; > unsigned nb_tx_queues; > - > int if_index; > - > - struct pcap_rx_queue rx_queue[RTE_PMD_RING_MAX_RX_RINGS]; > - struct pcap_tx_queue tx_queue[RTE_PMD_RING_MAX_TX_RINGS]; > + int single_iface; > }; >=20 > const char *valid_arguments[] =3D { > @@ -106,6 +113,10 @@ const char *valid_arguments[] =3D { > NULL > }; >=20 > +static int open_single_tx_pcap(const char *pcap_filename, > pcap_dumper_t **dumper); > +static int open_single_rx_pcap(const char *pcap_filename, pcap_t=20 > **pcap); > +static int open_single_iface(const char *iface, pcap_t **pcap); > + > static struct ether_addr eth_addr =3D { .addr_bytes =3D { 0, 0, 0, 0x1= , > 0x2, 0x3 } }; > static const char *drivername =3D "Pcap PMD"; > static struct rte_eth_link pmd_link =3D { > @@ -258,6 +269,59 @@ eth_pcap_tx(void *queue, > static int > eth_dev_start(struct rte_eth_dev *dev) > { > + unsigned i; > + struct pmd_internals *internals =3D dev->data->dev_private; > + struct pcap_tx_queue *tx; > + struct pcap_rx_queue *rx; > + > + /* Special iface case. Single pcap is open and shared between tx/rx.=20 > */ > + if (internals->single_iface) { > + tx =3D &internals->tx_queue[0]; > + rx =3D &internals->rx_queue[0]; > + > + if (!tx->pcap && strcmp(tx->type, ETH_PCAP_IFACE_ARG) =3D=3D 0) { > + if (open_single_iface(tx->name, &tx->pcap) < 0) > + return -1; > + rx->pcap =3D tx->pcap; > + } > + goto status_up; > + } > + > + /* If not open already, open tx pcaps/dumpers */ > + for (i =3D 0; i < internals->nb_tx_queues; i++) { > + tx =3D &internals->tx_queue[i]; > + > + if (!tx->dumper && strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) =3D=3D 0) = { > + if (open_single_tx_pcap(tx->name, &tx->dumper) < 0) > + return -1; > + } > + > + else if (!tx->pcap && strcmp(tx->type, ETH_PCAP_TX_IFACE_ARG) =3D=3D= 0)=20 > { > + if (open_single_iface(tx->name, &tx->pcap) < 0) > + return -1; > + } > + } > + > + /* If not open already, open rx pcaps */ > + for (i =3D 0; i < internals->nb_rx_queues; i++) { > + rx =3D &internals->rx_queue[i]; > + > + if (rx->pcap !=3D NULL) > + continue; > + > + if (strcmp(rx->type, ETH_PCAP_RX_PCAP_ARG) =3D=3D 0) { > + if (open_single_rx_pcap(rx->name, &rx->pcap) < 0) > + return -1; > + } > + > + else if (strcmp(rx->type, ETH_PCAP_RX_IFACE_ARG) =3D=3D 0) { > + if (open_single_iface(rx->name, &rx->pcap) < 0) > + return -1; > + } > + } > + > +status_up: > + > dev->data->dev_link.link_status =3D 1; > return 0; > } > @@ -271,19 +335,44 @@ static void > eth_dev_stop(struct rte_eth_dev *dev) > { > unsigned i; > - pcap_dumper_t *dumper; > - pcap_t *pcap; > struct pmd_internals *internals =3D dev->data->dev_private; > + struct pcap_tx_queue *tx; > + struct pcap_rx_queue *rx; > + > + /* Special iface case. Single pcap is open and shared between tx/rx.=20 > */ > + if (internals->single_iface) { > + tx =3D &internals->tx_queue[0]; > + rx =3D &internals->rx_queue[0]; > + pcap_close(tx->pcap); > + tx->pcap =3D NULL; > + rx->pcap =3D NULL; > + goto status_down; > + } >=20 > for (i =3D 0; i < internals->nb_tx_queues; i++) { > - dumper =3D internals->tx_queue[i].dumper; > - if(dumper !=3D NULL) > - pcap_dump_close(dumper); > - pcap =3D internals->tx_queue[i].pcap; > - if(pcap !=3D NULL) > - pcap_close(pcap); > + tx =3D &internals->tx_queue[i]; > + > + if (tx->dumper !=3D NULL) { > + pcap_dump_close(tx->dumper); > + tx->dumper =3D NULL; > + } > + > + if (tx->pcap !=3D NULL) { > + pcap_close(tx->pcap); > + tx->pcap =3D NULL; > + } > + } > + > + for (i =3D 0; i < internals->nb_rx_queues; i++) { > + rx =3D &internals->rx_queue[i]; > + > + if (rx->pcap !=3D NULL) { > + pcap_close(rx->pcap); > + rx->pcap =3D NULL; > + } > } >=20 > +status_down: > dev->data->dev_link.link_status =3D 0; > } >=20 > @@ -415,55 +504,79 @@ static struct eth_dev_ops ops =3D { > * reference of it for use it later on. > */ > static int > -open_rx_pcap(const char *key __rte_unused, const char *value, void=20 > *extra_args) > +open_rx_pcap(const char *key, const char *value, void *extra_args) > { > unsigned i; > const char *pcap_filename =3D value; > struct rx_pcaps *pcaps =3D extra_args; > - pcap_t *rx_pcap; > + pcap_t *pcap =3D NULL; >=20 > for (i =3D 0; i < pcaps->num_of_rx; i++) { > - if ((rx_pcap =3D pcap_open_offline(pcap_filename, errbuf)) =3D=3D NU= LL) { > - RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", pcap_filename, errbuf); > + if (open_single_rx_pcap(pcap_filename, &pcap) < 0) > return -1; > - } > - pcaps->pcaps[i] =3D rx_pcap; > + > + pcaps->pcaps[i] =3D pcap; > + pcaps->names[i] =3D pcap_filename; > + pcaps->types[i] =3D key; > } >=20 > return 0; > } >=20 > +static int > +open_single_rx_pcap(const char *pcap_filename, pcap_t **pcap) > +{ > + if ((*pcap =3D pcap_open_offline(pcap_filename, errbuf)) =3D=3D NULL)= { > + RTE_LOG(ERR, PMD, "Couldn't open %s: %s\n", pcap_filename, errbuf); > + return -1; > + } > + return 0; > +} > + > /* > * Opens a pcap file for writing and stores a reference to it > * for use it later on. > */ > static int > -open_tx_pcap(const char *key __rte_unused, const char *value, void=20 > *extra_args) > +open_tx_pcap(const char *key, const char *value, void *extra_args) > { > unsigned i; > const char *pcap_filename =3D value; > struct tx_pcaps *dumpers =3D extra_args; > - pcap_t *tx_pcap; > pcap_dumper_t *dumper; >=20 > for (i =3D 0; i < dumpers->num_of_tx; i++) { > - /* > - * We need to create a dummy empty pcap_t to use it > - * with pcap_dump_open(). We create big enough an Ethernet > - * pcap holder. > - */ > - if ((tx_pcap =3D pcap_open_dead(DLT_EN10MB,=20 > RTE_ETH_PCAP_SNAPSHOT_LEN)) > - =3D=3D NULL) { > - RTE_LOG(ERR, PMD, "Couldn't create dead pcap\n"); > + if (open_single_tx_pcap(pcap_filename, &dumper) < 0) > return -1; > - } >=20 > - /* The dumper is created using the previous pcap_t reference */ > - if ((dumper =3D pcap_dump_open(tx_pcap, pcap_filename)) =3D=3D NULL)= { > - RTE_LOG(ERR, PMD, "Couldn't open %s for writing.\n",=20 > pcap_filename); > - return -1; > - } > dumpers->dumpers[i] =3D dumper; > + dumpers->names[i] =3D pcap_filename; > + dumpers->types[i] =3D key; > + } > + > + return 0; > +} > + > +static int > +open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper) > +{ > + pcap_t *tx_pcap; > + /* > + * We need to create a dummy empty pcap_t to use it > + * with pcap_dump_open(). We create big enough an Ethernet > + * pcap holder. > + */ > + > + if ((tx_pcap =3D pcap_open_dead(DLT_EN10MB, RTE_ETH_PCAP_SNAPSHOT_LEN= )) > + =3D=3D NULL) { > + RTE_LOG(ERR, PMD, "Couldn't create dead pcap\n"); > + return -1; > + } > + > + /* The dumper is created using the previous pcap_t reference */ > + if ((*dumper =3D pcap_dump_open(tx_pcap, pcap_filename)) =3D=3D NULL)= { > + RTE_LOG(ERR, PMD, "Couldn't open %s for writing.\n", pcap_filename); > + return -1; > } >=20 > return 0; > @@ -488,13 +601,19 @@ open_iface_live(const char *iface, pcap_t **pcap)= =20 > { > * Opens an interface for reading and writing > */ > static inline int > -open_rx_tx_iface(const char *key __rte_unused, const char *value, > void *extra_args) > +open_rx_tx_iface(const char *key, const char *value, void *extra_args) > { > const char *iface =3D value; > - pcap_t **pcap =3D extra_args; > + struct rx_pcaps *pcaps =3D extra_args; > + pcap_t *pcap =3D NULL; >=20 > - if(open_iface_live(iface, pcap) < 0) > + if (open_single_iface(iface, &pcap) < 0) > return -1; > + > + pcaps->pcaps[0] =3D pcap; > + pcaps->names[0] =3D iface; > + pcaps->types[0] =3D key; > + > return 0; > } >=20 > @@ -502,7 +621,7 @@ open_rx_tx_iface(const char *key __rte_unused, > const char *value, void *extra_ar > * Opens a NIC for reading packets from it > */ > static inline int > -open_rx_iface(const char *key __rte_unused, const char *value, void > *extra_args) > +open_rx_iface(const char *key, const char *value, void *extra_args) > { > unsigned i; > const char *iface =3D value; > @@ -510,9 +629,11 @@ open_rx_iface(const char *key __rte_unused, const > char *value, void *extra_args) > pcap_t *pcap =3D NULL; >=20 > for (i =3D 0; i < pcaps->num_of_rx; i++) { > - if(open_iface_live(iface, &pcap) < 0) > + if (open_single_iface(iface, &pcap) < 0) > return -1; > pcaps->pcaps[i] =3D pcap; > + pcaps->names[i] =3D iface; > + pcaps->types[i] =3D key; > } >=20 > return 0; > @@ -521,8 +642,8 @@ open_rx_iface(const char *key __rte_unused, const > char *value, void *extra_args) > /* > * Opens a NIC for writing packets to it > */ > -static inline int > -open_tx_iface(const char *key __rte_unused, const char *value, void > *extra_args) > +static int > +open_tx_iface(const char *key, const char *value, void *extra_args) > { > unsigned i; > const char *iface =3D value; > @@ -530,14 +651,26 @@ open_tx_iface(const char *key __rte_unused, > const char *value, void *extra_args) > pcap_t *pcap; >=20 > for (i =3D 0; i < pcaps->num_of_tx; i++) { > - if(open_iface_live(iface, &pcap) < 0) > + if (open_single_iface(iface, &pcap) < 0) > return -1; > pcaps->pcaps[i] =3D pcap; > + pcaps->names[i] =3D iface; > + pcaps->types[i] =3D key; > } >=20 > return 0; > } >=20 > +static int > +open_single_iface(const char *iface, pcap_t **pcap) > +{ > + if (open_iface_live(iface, pcap) < 0) { > + RTE_LOG(ERR, PMD, "Couldn't open interface %s\n", iface); > + return -1; > + } > + > + return 0; > +} >=20 > static int > rte_pmd_init_internals(const char *name, const unsigned nb_rx_queues, > @@ -623,9 +756,10 @@ rte_pmd_init_internals(const char *name, const > unsigned nb_rx_queues, > } >=20 > static int > -rte_eth_from_pcaps_n_dumpers(const char *name, pcap_t * const=20 > rx_queues[], > +rte_eth_from_pcaps_n_dumpers(const char *name, > + struct rx_pcaps *rx_queues, > const unsigned nb_rx_queues, > - pcap_dumper_t * const tx_queues[], > + struct tx_pcaps *tx_queues, > const unsigned nb_tx_queues, > const unsigned numa_node, > struct rte_kvargs *kvlist) > @@ -645,25 +779,35 @@ rte_eth_from_pcaps_n_dumpers(const char *name, > pcap_t * const rx_queues[], > return -1; >=20 > for (i =3D 0; i < nb_rx_queues; i++) { > - internals->rx_queue->pcap =3D rx_queues[i]; > + internals->rx_queue->pcap =3D rx_queues->pcaps[i]; > + internals->rx_queue->name =3D rx_queues->names[i]; > + internals->rx_queue->type =3D rx_queues->types[i]; > } > for (i =3D 0; i < nb_tx_queues; i++) { > - internals->tx_queue->dumper =3D tx_queues[i]; > + internals->tx_queue->dumper =3D tx_queues->dumpers[i]; > + internals->tx_queue->name =3D tx_queues->names[i]; > + internals->tx_queue->type =3D tx_queues->types[i]; > } >=20 > + /* using multiple pcaps/interfaces */ > + internals->single_iface =3D 0; > + > eth_dev->rx_pkt_burst =3D eth_pcap_rx; > eth_dev->tx_pkt_burst =3D eth_pcap_tx_dumper; >=20 > return 0; > } >=20 > + struct rx_pcaps pcaps; > static int > -rte_eth_from_pcaps(const char *name, pcap_t * const rx_queues[], > +rte_eth_from_pcaps(const char *name, > + struct rx_pcaps *rx_queues, > const unsigned nb_rx_queues, > - pcap_t * const tx_queues[], > + struct tx_pcaps *tx_queues, > const unsigned nb_tx_queues, > const unsigned numa_node, > - struct rte_kvargs *kvlist) > + struct rte_kvargs *kvlist, > + int single_iface) > { > struct pmd_internals *internals =3D NULL; > struct rte_eth_dev *eth_dev =3D NULL; > @@ -680,12 +824,19 @@ rte_eth_from_pcaps(const char *name, pcap_t * > const rx_queues[], > return -1; >=20 > for (i =3D 0; i < nb_rx_queues; i++) { > - internals->rx_queue->pcap =3D rx_queues[i]; > + internals->rx_queue->pcap =3D rx_queues->pcaps[i]; > + internals->rx_queue->name =3D rx_queues->names[i]; > + internals->rx_queue->type =3D rx_queues->types[i]; > } > for (i =3D 0; i < nb_tx_queues; i++) { > - internals->tx_queue->pcap =3D tx_queues[i]; > + internals->tx_queue->pcap =3D tx_queues->pcaps[i]; > + internals->tx_queue->name =3D tx_queues->names[i]; > + internals->tx_queue->type =3D tx_queues->types[i]; > } >=20 > + /* store wether we are using a single interface for rx/tx or not */ > + internals->single_iface =3D single_iface; > + > eth_dev->rx_pkt_burst =3D eth_pcap_rx; > eth_dev->tx_pkt_burst =3D eth_pcap_tx; >=20 > @@ -721,12 +872,14 @@ rte_pmd_pcap_devinit(const char *name, const char= =20 > *params) > if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) =3D=3D 1) { >=20 > ret =3D rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG, > - &open_rx_tx_iface, &pcaps.pcaps[0]); > + &open_rx_tx_iface, &pcaps); > if (ret < 0) > return -1; > - > - return rte_eth_from_pcaps(name, pcaps.pcaps, 1, pcaps.pcaps, 1, > - numa_node, kvlist); > + dumpers.pcaps[0] =3D pcaps.pcaps[0]; > + dumpers.names[0] =3D pcaps.names[0]; > + dumpers.types[0] =3D pcaps.types[0]; > + return rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1, > + numa_node, kvlist, 1); > } >=20 > /* > @@ -766,11 +919,11 @@ rte_pmd_pcap_devinit(const char *name, const char= =20 > *params) > return -1; >=20 > if (using_dumpers) > - return rte_eth_from_pcaps_n_dumpers(name, pcaps.pcaps,=20 > pcaps.num_of_rx, > - dumpers.dumpers, dumpers.num_of_tx, numa_node, kvlist); > + return rte_eth_from_pcaps_n_dumpers(name, &pcaps, pcaps.num_of_rx, > + &dumpers, dumpers.num_of_tx, numa_node, kvlist); >=20 > - return rte_eth_from_pcaps(name, pcaps.pcaps, pcaps.num_of_rx,=20 > dumpers.pcaps, > - dumpers.num_of_tx, numa_node, kvlist); > + return rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_rx, &dumpers, > + dumpers.num_of_tx, numa_node, kvlist, 0); >=20 > }