[PATCH] ethdev: add template table resize API
Template table creation API sets table flows capacity.
If application needs more flows then the table was designed for,
the following procedures must be completed:
1. Create a new template table with larger flows capacity.
2. Re-create existing flows in the new table and delete flows from
the original table.
3. Destroy original table.
Application cannot always execute that procedure:
* Port may not have sufficient resources to allocate a new table
while maintaining original table.
* Application may not have existing flows "recipes" to re-create
flows in a new table.
The patch defines a new API that allows application to resize
existing template table:
* Resizable template table must be created with the
RTE_FLOW_TABLE_SPECIALIZE_RESIZABLE_TABLE bit set.
* Application resizes existing table with the
`rte_flow_template_table_resize()` function call.
The table resize procedure updates the table maximal flow number
only. Other table attributes are not affected by the table resize.
** The table resize procedure must not interrupt
existing table flows operations in hardware.
** The table resize procedure must not alter flow handlers held by
application.
* After `rte_flow_template_table_resize()` returned, application must
update all existing table flow rules by calling
`rte_flow_async_update_resized()`.
The table resize procedure does not change application flow handler.
However, flow object can reference internal PMD resources that are
obsolete after table resize.
`rte_flow_async_update_resized()` moves internal flow references
to the updated table resources.
The flow update must not interrupt hardware flow operations.
* When all table flow were updated, application must call
`rte_flow_template_table_resize_complete()`.
The function releases PMD resources related to the original
table.
Application can start new table resize after
`rte_flow_template_table_resize_complete()` returned.
Signed-off-by: Gregory Etelson
Acked-by: Ori Kam
---
app/test-pmd/cmdline_flow.c| 86 +++--
app/test-pmd/config.c | 102 +
app/test-pmd/testpmd.h | 6 ++
doc/guides/rel_notes/release_24_03.rst | 2 +
lib/ethdev/ethdev_trace.h | 33
lib/ethdev/ethdev_trace_points.c | 9 +++
lib/ethdev/rte_flow.c | 69 +
lib/ethdev/rte_flow.h | 97 +++
lib/ethdev/rte_flow_driver.h | 15
lib/ethdev/version.map | 3 +
10 files changed, 417 insertions(+), 5 deletions(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index ce71818705..a34757a13e 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -134,6 +134,7 @@ enum index {
/* Queue arguments. */
QUEUE_CREATE,
QUEUE_DESTROY,
+ QUEUE_FLOW_UPDATE_RESIZED,
QUEUE_UPDATE,
QUEUE_AGED,
QUEUE_INDIRECT_ACTION,
@@ -191,8 +192,12 @@ enum index {
/* Table arguments. */
TABLE_CREATE,
TABLE_DESTROY,
+ TABLE_RESIZE,
+ TABLE_RESIZE_COMPLETE,
TABLE_CREATE_ID,
TABLE_DESTROY_ID,
+ TABLE_RESIZE_ID,
+ TABLE_RESIZE_RULES_NUMBER,
TABLE_INSERTION_TYPE,
TABLE_INSERTION_TYPE_NAME,
TABLE_HASH_FUNC,
@@ -204,6 +209,7 @@ enum index {
TABLE_TRANSFER,
TABLE_TRANSFER_WIRE_ORIG,
TABLE_TRANSFER_VPORT_ORIG,
+ TABLE_RESIZABLE,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -1323,6 +1329,8 @@ static const enum index next_group_attr[] = {
static const enum index next_table_subcmd[] = {
TABLE_CREATE,
TABLE_DESTROY,
+ TABLE_RESIZE,
+ TABLE_RESIZE_COMPLETE,
ZERO,
};
@@ -1337,6 +1345,7 @@ static const enum index next_table_attr[] = {
TABLE_TRANSFER,
TABLE_TRANSFER_WIRE_ORIG,
TABLE_TRANSFER_VPORT_ORIG,
+ TABLE_RESIZABLE,
TABLE_RULES_NUMBER,
TABLE_PATTERN_TEMPLATE,
TABLE_ACTIONS_TEMPLATE,
@@ -1353,6 +1362,7 @@ static const enum index next_table_destroy_attr[] = {
static const enum index next_queue_subcmd[] = {
QUEUE_CREATE,
QUEUE_DESTROY,
+ QUEUE_FLOW_UPDATE_RESIZED,
QUEUE_UPDATE,
QUEUE_AGED,
QUEUE_INDIRECT_ACTION,
@@ -3344,6 +3354,19 @@ static const struct token token_list[] = {
.args = ARGS(ARGS_ENTRY(struct buffer, port)),
.call = parse_table_destroy,
},
+ [TABLE_RESIZE] = {
+ .name = "resize",
+ .help = "resize template table",
+ .next = NEXT(NEXT_ENTRY(TABLE_RESIZE_ID)),
+ .call = parse_table
+ },
+ [TABLE_RESIZE_COMPLETE] = {
+ .name = "resize_complete",
+ .help = "complete table resize",
+ .next = N
RE: [RFC] ethdev: introduce entropy calculation
Hi Andrew,
> -Original Message-
> From: Andrew Rybchenko
> Sent: Saturday, December 16, 2023 11:19 AM
>
> On 12/10/23 11:30, Ori Kam wrote:
> > When offloading rules with the encap action, the HW may calculate entropy
> based on the encap protocol.
> > Each HW can implement a different algorithm.
> >
> > When the application receives packets that should have been
> > encaped by the HW, but didn't reach this stage yet (for example TCP SYN
> packets),
> > then when encap is done in SW, application must apply
> > the same entropy calculation algorithm.
> >
> > Using the new API application can request the PMD to calculate the
> > value as if the packet passed in the HW.
>
> I'm still wondering why the API is required. Does app install encap
> rule when the first packet is processed? The rule can contain all
> outer headers (as it is calculated in SW anyway) and HW does not need
> to calculate anything.
Yes, the application installs a rule based on the first packet.
as a result, all the rest of the packets are encaped by the HW.
This API allows the application to use the same value as the HW will use when
encapsulating the packet.
In other words, we have 2 paths:
Path 1 SW, for the first packet
Path 2 HW, all the rest of the packest
>
> >
> > Signed-off-by: Ori Kam
> > ---
> > lib/ethdev/rte_flow.h | 49
> +++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index affdc8121b..3989b089dd 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -6753,6 +6753,55 @@ rte_flow_calc_table_hash(uint16_t port_id, const
> struct rte_flow_template_table
> > const struct rte_flow_item pattern[], uint8_t
> pattern_template_index,
> > uint32_t *hash, struct rte_flow_error *error);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Destination field type for the entropy calculation.
> > + *
> > + * @see function rte_flow_calc_encap_entropy
> > + */
> > +enum rte_flow_entropy_dest {
> > + /* Calculate entropy placed in UDP source port field. */
> > + RTE_FLOW_ENTROPY_DEST_UDP_SRC_PORT,
>
> And source and destination are used together but for different
> purposes it is very hard to follow which is used for which purpose.
> I'd avoid term "dest" in the enum naming. May be present "flow" is
> already good enough to highlight that it is per-flow?
> rte_flow_encap_hash? rte_flow_encap_field_hash?
I'm open to any suggestions, this enum is supposed to show to which
field the HW insert the calculated value. This field is defined by the
encapsulation
protocol. For example, VXLAN the hash is stored in source port, while in NVGRE
it is stored in
flow_id field. The destination field also impact the size.
What do you think about:
RTE_FLOW_ENCAP_HASH_SRC_PORT?
What about if we change the destination to enum that will hold the destination
tunnel type
RTE_FLOW_TUNNEL_TYPE_VXLAN,
RTE_FLOW_TUNNEL_TYPE_NVGRE
>
> > + /* Calculate entropy placed in NVGRE flow ID field. */
> > + RTE_FLOW_ENTROPY_DEST_NVGRE_FLOW_ID,
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Calculate the entropy generated by the HW for a given pattern,
> > + * when encapsulation flow action is executed.
> > + *
> > + * @param[in] port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] pattern
> > + * The values to be used in the entropy calculation.
>
> Is it inner packet fields? Should all fields be used for hash
> calculation? May be it is better to describe as inner packet
> headers? How does app know which headers to extract and provide?
The fields are the outer fields that will become inner fields, after the
encapsulation.
The fields are dependent on the HW (in standard case 5 tuple). but applications
can /should set
all the fields he got from the packet, at the end those are the fields that the
HW will see.
>
> > + * @param[in] dest_field
> > + * Type of destination field for entropy calculation.
> > + * @param[out] entropy
> > + * Used to return the calculated entropy. It will be written in network
> > order,
> > + * so entropy[0] is the MSB.
> > + * The number of bytes is based on the destination field type.f
>
> API contract is a bit unclear here. May be it is safer to provide
> buffer size explicitly?
The size of the field is defined by the destination field, which in turn is
defined by the
protocol.
I don't think adding size has any meaning when you know that the value is going
to be set
in source port which has the size of 16 bites.
Based on enum suggestion of tunnel type. I think it will also explain the dest
and size. What do you think?
>
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + * PMDs initialize this structure in case of error only.
> > +
[PATCH] net/e1000: support launchtime feature
Enable the time-based scheduled Tx of packets based on the
RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP flag. The launchtime defines the
packet transmission time based on PTP clock at MAC layer, which should
be set to the advanced transmit descriptor.
Signed-off-by: Chuanyu Xue
---
drivers/net/e1000/base/e1000_regs.h | 1 +
drivers/net/e1000/e1000_ethdev.h| 3 ++
drivers/net/e1000/igb_ethdev.c | 28 ++
drivers/net/e1000/igb_rxtx.c| 44 -
4 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/net/e1000/base/e1000_regs.h
b/drivers/net/e1000/base/e1000_regs.h
index d44de59c29..092d9d71e6 100644
--- a/drivers/net/e1000/base/e1000_regs.h
+++ b/drivers/net/e1000/base/e1000_regs.h
@@ -162,6 +162,7 @@
/* QAV Tx mode control register */
#define E1000_I210_TQAVCTRL0x3570
+#define E1000_I210_LAUNCH_OS0 0x3578
/* QAV Tx mode control register bitfields masks */
/* QAV enable */
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 718a9746ed..174f7aaf52 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -382,6 +382,9 @@ extern struct igb_rss_filter_list igb_filter_rss_list;
TAILQ_HEAD(igb_flow_mem_list, igb_flow_mem);
extern struct igb_flow_mem_list igb_flow_list;
+extern uint64_t igb_tx_timestamp_dynflag;
+extern int igb_tx_timestamp_dynfield_offset;
+
extern const struct rte_flow_ops igb_flow_ops;
/*
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8..4d3d8ae30a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -223,6 +223,7 @@ static int igb_timesync_read_time(struct rte_eth_dev *dev,
struct timespec *timestamp);
static int igb_timesync_write_time(struct rte_eth_dev *dev,
const struct timespec *timestamp);
+static int eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t
*clock);
static int eth_igb_rx_queue_intr_enable(struct rte_eth_dev *dev,
uint16_t queue_id);
static int eth_igb_rx_queue_intr_disable(struct rte_eth_dev *dev,
@@ -313,6 +314,9 @@ static const struct rte_pci_id pci_id_igbvf_map[] = {
{ .vendor_id = 0, /* sentinel */ },
};
+uint64_t igb_tx_timestamp_dynflag;
+int igb_tx_timestamp_dynfield_offset = -1;
+
static const struct rte_eth_desc_lim rx_desc_lim = {
.nb_max = E1000_MAX_RING_DESC,
.nb_min = E1000_MIN_RING_DESC,
@@ -389,6 +393,7 @@ static const struct eth_dev_ops eth_igb_ops = {
.timesync_adjust_time = igb_timesync_adjust_time,
.timesync_read_time = igb_timesync_read_time,
.timesync_write_time = igb_timesync_write_time,
+ .read_clock = eth_igb_read_clock,
};
/*
@@ -1198,6 +1203,7 @@ eth_igb_start(struct rte_eth_dev *dev)
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
int ret, mask;
+ uint32_t tqavctrl;
uint32_t intr_vector = 0;
uint32_t ctrl_ext;
uint32_t *speeds;
@@ -1281,6 +1287,15 @@ eth_igb_start(struct rte_eth_dev *dev)
return ret;
}
+ if (igb_tx_timestamp_dynflag > 0) {
+ tqavctrl = E1000_READ_REG(hw, E1000_I210_TQAVCTRL);
+ tqavctrl |= E1000_TQAVCTRL_MODE;
+ tqavctrl |= E1000_TQAVCTRL_FETCH_ARB; /* Fetch the queue most
empty, no Round Robin*/
+ tqavctrl |= E1000_TQAVCTRL_LAUNCH_TIMER_ENABLE; /* Enable
launch time */
+ E1000_WRITE_REG(hw, E1000_I210_TQAVCTRL, tqavctrl);
+ E1000_WRITE_REG(hw, E1000_I210_LAUNCH_OS0, 1ULL << 31); /* Set
launch offset to default */
+ }
+
e1000_clear_hw_cntrs_base_generic(hw);
/*
@@ -4882,6 +4897,19 @@ igb_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
return 0;
}
+static int
+eth_igb_read_clock(__rte_unused struct rte_eth_dev *dev, uint64_t *clock)
+{
+ uint64_t systime_cycles;
+ struct e1000_adapter *adapter = dev->data->dev_private;
+
+ systime_cycles = igb_read_systime_cyclecounter(dev);
+ uint64_t ns = rte_timecounter_update(&adapter->systime_tc,
systime_cycles);
+ *clock = ns;
+
+ return 0;
+}
+
static int
eth_igb_get_reg_length(struct rte_eth_dev *dev __rte_unused)
{
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 448c4b7d9d..e5da8e250d 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -212,6 +212,9 @@ struct igb_tx_queue {
#define IGB_TSO_MAX_HDRLEN (512)
#define IGB_TSO_MAX_MSS(9216)
+/* Macro to compensate latency in launch time offloading*/
+#define E1000_I210_LT_LATENCY 0x41F9
+
/*
*
* TX func
RE: [PATCH 2/3] net/nfp: fix free resource problem
> On 12/14/2023 10:24 AM, Chaoyong He wrote:
> > From: Long Wu
> >
> > Set the representor array to NULL to avoid that close interface does
> > not free some resource.
> >
> > Fixes: a135bc1644d6 ("net/nfp: fix resource leak for flower firmware")
> > Cc: chaoyong...@corigine.com
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Long Wu
> > Reviewed-by: Chaoyong He
> > Reviewed-by: Peng Zhang
> > ---
> > drivers/net/nfp/flower/nfp_flower_representor.c | 15 ++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/nfp/flower/nfp_flower_representor.c
> > b/drivers/net/nfp/flower/nfp_flower_representor.c
> > index 27ea3891bd..5f7c1fa737 100644
> > --- a/drivers/net/nfp/flower/nfp_flower_representor.c
> > +++ b/drivers/net/nfp/flower/nfp_flower_representor.c
> > @@ -294,17 +294,30 @@ nfp_flower_repr_tx_burst(void *tx_queue, static
> > int nfp_flower_repr_uninit(struct rte_eth_dev *eth_dev) {
> > + uint16_t index;
> > struct nfp_flower_representor *repr;
> >
> > repr = eth_dev->data->dev_private;
> > rte_ring_free(repr->ring);
> >
> > + if (repr->repr_type == NFP_REPR_TYPE_PHYS_PORT) {
> > + index = NFP_FLOWER_CMSG_PORT_PHYS_PORT_NUM(repr-
> >port_id);
> > + repr->app_fw_flower->phy_reprs[index] = NULL;
> > + } else {
> > + index = repr->vf_id;
> > + repr->app_fw_flower->vf_reprs[index] = NULL;
> > + }
> > +
> > return 0;
> > }
> >
> > static int
> > -nfp_flower_pf_repr_uninit(__rte_unused struct rte_eth_dev *eth_dev)
> > +nfp_flower_pf_repr_uninit(struct rte_eth_dev *eth_dev)
> > {
> > + struct nfp_flower_representor *repr = eth_dev->data->dev_private;
> > +
> > + repr->app_fw_flower->pf_repr = NULL;
> >
>
> Here it is assigned to NULL but is it freed? If freed, why not set to NULL
> where
> it is freed?
>
> Same for above phy_reprs & vf_reprs.
The whole invoke view:
rte_eth_dev_close()
--> nfp_flower_repr_dev_close()
--> nfp_flower_repr_free()
--> nfp_flower_pf_repr_uninit()
--> nfp_flower_repr_uninit()
// In these two functions, we just assigned to NULL but not freed
yet.
// It is still refer by the `eth_dev->data->dev_private`.
--> rte_eth_dev_release_port()
--> rte_free(eth_dev->data->dev_private);
// And here it is really freed (by the rte framework).
[BUG] [bonding] bonding member delete bug
Hi all,
I'm using DPDK-21.11 in ovs-dpdk.
I found a "bonding member delete bug" .
1. How to reproduce
```
NOTICE: bondctl is a tool I develop, it's to control DPDK.
### step 1, Add bonding device bond0.
bondctl add bond0 mode active-backup
### step 2, Add member m1 into bond0.
bondctl set :00:0a.0 master bond0
### step 3, Add bond0 into ovs bridge.
ovs-vsctl add-port brp0 bond0 -- set interface bond0 type=dpdk
options:dpdk-devargs=net_bonding-bond0
(this command call @bond_ethdev_start at last.)
### step 4, Delete bond0 from ovs bridge.
ovs-vsctl del-port br-phy bond0
(this command call @bond_ethdev_stop at last.)
### step 5, Delete m1 from bond0.
bondctl set :00:0a.0 nomaster
### step 6, Delete bond0.
bondctl del bond0
### step 7, Add bond0.
bondctl add bond0 mode active-backup
### step 8, Add member m1 into bond0.
bondctl set :00:0a.0 master bond0
(this command call @bond_ethdev_start at last.)
### Then got error message.
2023-12-15T08:24:04.153Z|00017|dpdk|ERR|Port 0 must be stopped to allow
configurr
ation
2023-12-15T08:24:04.153Z|00018|dpdk|ERR|bond_cmd_set_master(581) - can not
confii
g slave :00:0a.0!
```
2. Debug
I found the reason is, when member port is DOWN, then add operation will
call "eth_dev->data->dev_started = 1;", but no one add active member port,
so when delete bond0, will NOT call @rte_eth_dev_stop, then add bond0
again, got error. Detail is:
```
### After step 1-3, add bond0 into ovs-dpdk
bond_ethdev_start
eth_dev->data->dev_started = 1;
for (i = 0; i < internals->slave_count; i++) {
if (slave_configure(eth_dev, slave_ethdev) != 0) {
if (slave_start(eth_dev, slave_ethdev) != 0) {
rte_eth_dev_start
### NOTICE, as member port is DOWN, so will NOT call @activate_slave,
so @active_slave_count is 0.
bond_ethdev_lsc_event_callback
activate_slave(bonded_eth_dev, port_id);
### After step 4, delete bond0 from ovs-dpdk, NOTICE,
as @active_slave_count is 0, so will NOT call @rte_eth_dev_stop
bond_ethdev_stop
for (i = 0; i < internals->slave_count; i++) {
if (find_slave_by_id(internals->active_slaves,
internals->active_slave_count, slave_id) !=
internals->active_slave_count) {
ret = rte_eth_dev_stop(slave_id);
### After step 5-7, delete bond0 and then add bond0
### After step 8, add bond0, as it's NOT call @rte_eth_dev_stop, so
call @rte_eth_dev_start
again will got error.
2023-12-15T08:24:04.153Z|00017|dpdk|ERR|Port 0 must be stopped to allow
configurr
ation
```
3. My question
Is this bug fixed ? Which commit ?
If NOT, how to fix this bug? I think it's better to call @rte_eth_dev_stop
for every member, even it's DOWN. How about this?
Thanks~
Simon Jones
RE: [PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API
[AMD Official Use Only - General]
> -Original Message-
> From: Euan Bourke
> Sent: Friday, December 15, 2023 10:57 PM
> To: dev@dpdk.org
> Cc: Euan Bourke ; David Hunt ;
> Anatoly Burakov ; Tummala, Sivaprasad
>
> Subject: [PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Update to the l3fwd-power example application to call the arg parser library
> for its
> 'combined core string parser' instead of implementing its own corelist
> parser. The
> default_type passed into the function call is a corelist.
>
> Signed-off-by: Euan Bourke
> Acked-by: David Hunt
> ---
> examples/l3fwd-power/perf_core.c | 52 ++--
> 1 file changed, 9 insertions(+), 43 deletions(-)
>
> diff --git a/examples/l3fwd-power/perf_core.c
> b/examples/l3fwd-power/perf_core.c
> index 41ef6d0c9a..e8ed414d40 100644
> --- a/examples/l3fwd-power/perf_core.c
> +++ b/examples/l3fwd-power/perf_core.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #include "perf_core.h"
> #include "main.h"
> @@ -177,56 +178,21 @@ parse_perf_config(const char *q_arg) int
> parse_perf_core_list(const char *corelist) {
> - int i, idx = 0;
> - unsigned int count = 0;
> - char *end = NULL;
> - int min, max;
> + int count;
> + uint16_t cores[RTE_MAX_LCORE];
>
> if (corelist == NULL) {
> printf("invalid core list\n");
> return -1;
> }
>
> + count = rte_arg_parse_core_string(corelist, cores, RTE_DIM(cores),
> + RTE_ARG_PARSE_TYPE_CORELIST);
>
> - /* Remove all blank characters ahead and after */
> - while (isblank(*corelist))
> - corelist++;
> - i = strlen(corelist);
> - while ((i > 0) && isblank(corelist[i - 1]))
> - i--;
> + for (int i = 0; i < count; i++)
> + hp_lcores[i] = cores[i];
>
> - /* Get list of cores */
> - min = RTE_MAX_LCORE;
> - do {
> - while (isblank(*corelist))
> - corelist++;
> - if (*corelist == '\0')
> - return -1;
> - errno = 0;
> - idx = strtoul(corelist, &end, 10);
> - if (errno || end == NULL)
> - return -1;
> - while (isblank(*end))
> - end++;
> - if (*end == '-') {
> - min = idx;
> - } else if ((*end == ',') || (*end == '\0')) {
> - max = idx;
> - if (min == RTE_MAX_LCORE)
> - min = idx;
> - for (idx = min; idx <= max; idx++) {
> - hp_lcores[count] = idx;
> - count++;
> - }
> - min = RTE_MAX_LCORE;
> - } else {
> - printf("invalid core list\n");
> - return -1;
> - }
> - corelist = end + 1;
> - } while (*end != '\0');
> -
> - if (count == 0) {
> + if (count == 0 || count == -1) {
> printf("invalid core list\n");
> return -1;
> }
> @@ -234,7 +200,7 @@ parse_perf_core_list(const char *corelist)
> nb_hp_lcores = count;
>
> printf("Configured %d high performance cores\n", nb_hp_lcores);
> - for (i = 0; i < nb_hp_lcores; i++)
> + for (int i = 0; i < nb_hp_lcores; i++)
> printf("\tHigh performance core %d %d\n",
> i, hp_lcores[i]);
>
> --
> 2.34.1
Acked-by: Sivaprasad Tummala
RE: [PATCH v2 RESEND] net/ixgbe: fix memoy leak after device init failure
> -Original Message-
> From: Yunjian Wang
> Sent: Monday, December 18, 2023 11:08 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z ; Yang, Qiming ;
> Wu, Wenjun1 ; xudin...@huawei.com;
> jerry.lili...@huawei.com; Yunjian Wang ;
> sta...@dpdk.org
> Subject: [PATCH v2 RESEND] net/ixgbe: fix memoy leak after device init failure
>
> In ixgbe_ipsec_ctx_create() allocated memory for the 'security_ctx', we should
> free it when errors occur, otherwise it will lead to memory leak.
>
> Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Yunjian Wang
Acked-by: Qi Zhang
Applied to dpdk-next-net-intel.
Thanks
Qi
Re: [BUG] [bonding] bonding member delete bug
Oh, it's fixed by 0911d4ec and f5e72e8e
Simon Jones
Simon Jones 于2023年12月18日周一 10:51写道：
> Hi all,
>
> I'm using DPDK-21.11 in ovs-dpdk.
>
> I found a "bonding member delete bug" .
>
> 1. How to reproduce
>
> ```
> NOTICE: bondctl is a tool I develop, it's to control DPDK.
>
> ### step 1, Add bonding device bond0.
> bondctl add bond0 mode active-backup
>
> ### step 2, Add member m1 into bond0.
> bondctl set :00:0a.0 master bond0
>
> ### step 3, Add bond0 into ovs bridge.
> ovs-vsctl add-port brp0 bond0 -- set interface bond0 type=dpdk
> options:dpdk-devargs=net_bonding-bond0
> (this command call @bond_ethdev_start at last.)
>
> ### step 4, Delete bond0 from ovs bridge.
> ovs-vsctl del-port br-phy bond0
> (this command call @bond_ethdev_stop at last.)
>
> ### step 5, Delete m1 from bond0.
> bondctl set :00:0a.0 nomaster
>
> ### step 6, Delete bond0.
> bondctl del bond0
>
> ### step 7, Add bond0.
> bondctl add bond0 mode active-backup
>
> ### step 8, Add member m1 into bond0.
> bondctl set :00:0a.0 master bond0
> (this command call @bond_ethdev_start at last.)
>
> ### Then got error message.
> 2023-12-15T08:24:04.153Z|00017|dpdk|ERR|Port 0 must be stopped to allow
> configurr
> ation
> 2023-12-15T08:24:04.153Z|00018|dpdk|ERR|bond_cmd_set_master(581) - can not
> confii
> g slave :00:0a.0!
> ```
>
> 2. Debug
>
> I found the reason is, when member port is DOWN, then add operation will
> call "eth_dev->data->dev_started = 1;", but no one add active member port,
> so when delete bond0, will NOT call @rte_eth_dev_stop, then add bond0
> again, got error. Detail is:
> ```
> ### After step 1-3, add bond0 into ovs-dpdk
> bond_ethdev_start
> eth_dev->data->dev_started = 1;
> for (i = 0; i < internals->slave_count; i++) {
> if (slave_configure(eth_dev, slave_ethdev) != 0) {
> if (slave_start(eth_dev, slave_ethdev) != 0) {
> rte_eth_dev_start
>
> ### NOTICE, as member port is DOWN, so will NOT call @activate_slave,
> so @active_slave_count is 0.
> bond_ethdev_lsc_event_callback
> activate_slave(bonded_eth_dev, port_id);
>
> ### After step 4, delete bond0 from ovs-dpdk, NOTICE,
> as @active_slave_count is 0, so will NOT call @rte_eth_dev_stop
> bond_ethdev_stop
> for (i = 0; i < internals->slave_count; i++) {
> if (find_slave_by_id(internals->active_slaves,
> internals->active_slave_count, slave_id) !=
> internals->active_slave_count) {
> ret = rte_eth_dev_stop(slave_id);
>
> ### After step 5-7, delete bond0 and then add bond0
>
> ### After step 8, add bond0, as it's NOT call @rte_eth_dev_stop, so call
> @rte_eth_dev_start
> again will got error.
> 2023-12-15T08:24:04.153Z|00017|dpdk|ERR|Port 0 must be stopped to allow
> configurr
> ation
>
> ```
>
> 3. My question
>
> Is this bug fixed ? Which commit ?
>
> If NOT, how to fix this bug? I think it's better to call @rte_eth_dev_stop
> for every member, even it's DOWN. How about this?
>
> Thanks~
>
>
>
> Simon Jones
>
[PATCH 1/6] examples/l3fwd: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/l3fwd/main.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 3bf28aec0c..847ded0ad2 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -99,7 +99,7 @@ struct parm_cfg parm_config;
struct lcore_params {
uint16_t port_id;
uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t lcore_id;
} __rte_cache_aligned;
static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
@@ -292,8 +292,8 @@ setup_l3fwd_lookup_tables(void)
static int
check_lcore_params(void)
{
- uint8_t queue, lcore;
- uint16_t i;
+ uint8_t queue;
+ uint16_t i, lcore;
int socketid;
for (i = 0; i < nb_lcore_params; ++i) {
@@ -359,7 +359,7 @@ static int
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint16_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -500,6 +500,8 @@ parse_config(const char *q_arg)
char *str_fld[_NUM_FLD];
int i;
unsigned size;
+ unsigned int max_fld[_NUM_FLD] = {RTE_MAX_ETHPORTS,
+ 255, RTE_MAX_LCORE};
nb_lcore_params = 0;
@@ -518,7 +520,8 @@ parse_config(const char *q_arg)
for (i = 0; i < _NUM_FLD; i++){
errno = 0;
int_fld[i] = strtoul(str_fld[i], &end, 0);
- if (errno != 0 || end == str_fld[i] || int_fld[i] > 255)
+ if (errno != 0 || end == str_fld[i] || int_fld[i] >
+
max_fld[i])
return -1;
}
if (nb_lcore_params >= MAX_LCORE_PARAMS) {
@@ -531,7 +534,7 @@ parse_config(const char *q_arg)
lcore_params_array[nb_lcore_params].queue_id =
(uint8_t)int_fld[FLD_QUEUE];
lcore_params_array[nb_lcore_params].lcore_id =
- (uint8_t)int_fld[FLD_LCORE];
+ (uint16_t)int_fld[FLD_LCORE];
++nb_lcore_params;
}
lcore_params = lcore_params_array;
--
2.25.1
[PATCH 2/6] examples/l3fwd-power: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: f88e7c175a68 ("examples/l3fwd-power: add high/regular perf cores
options")
Cc: radu.nico...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/l3fwd-power/main.c | 12 +++-
examples/l3fwd-power/main.h | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index f4adcf41b5..1f0ac3e660 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1399,8 +1399,8 @@ main_legacy_loop(__rte_unused void *dummy)
static int
check_lcore_params(void)
{
- uint8_t queue, lcore;
- uint16_t i;
+ uint8_t queue;
+ uint16_t lcore, i;
int socketid;
for (i = 0; i < nb_lcore_params; ++i) {
@@ -1469,7 +1469,7 @@ static int
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint16_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -1661,6 +1661,8 @@ parse_config(const char *q_arg)
char *str_fld[_NUM_FLD];
int i;
unsigned size;
+ unsigned int max_fld[_NUM_FLD] = {RTE_MAX_ETHPORTS,
+ 255, RTE_MAX_LCORE};
nb_lcore_params = 0;
@@ -1681,7 +1683,7 @@ parse_config(const char *q_arg)
errno = 0;
int_fld[i] = strtoul(str_fld[i], &end, 0);
if (errno != 0 || end == str_fld[i] || int_fld[i] >
- 255)
+
max_fld[i])
return -1;
}
if (nb_lcore_params >= MAX_LCORE_PARAMS) {
@@ -1694,7 +1696,7 @@ parse_config(const char *q_arg)
lcore_params_array[nb_lcore_params].queue_id =
(uint8_t)int_fld[FLD_QUEUE];
lcore_params_array[nb_lcore_params].lcore_id =
- (uint8_t)int_fld[FLD_LCORE];
+ (uint16_t)int_fld[FLD_LCORE];
++nb_lcore_params;
}
lcore_params = lcore_params_array;
diff --git a/examples/l3fwd-power/main.h b/examples/l3fwd-power/main.h
index 258de98f5b..4e5fd3b6d6 100644
--- a/examples/l3fwd-power/main.h
+++ b/examples/l3fwd-power/main.h
@@ -10,7 +10,7 @@
struct lcore_params {
uint16_t port_id;
uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t lcore_id;
} __rte_cache_aligned;
extern struct lcore_params *lcore_params;
--
2.25.1
[PATCH 3/6] examples/l3fwd-graph: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: 08bd1a174461 ("examples/l3fwd-graph: add graph-based l3fwd skeleton")
Cc: ndabilpu...@marvell.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/l3fwd-graph/main.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/examples/l3fwd-graph/main.c b/examples/l3fwd-graph/main.c
index 96cb1c81ff..149677a3cc 100644
--- a/examples/l3fwd-graph/main.c
+++ b/examples/l3fwd-graph/main.c
@@ -111,7 +111,7 @@ static struct lcore_conf lcore_conf[RTE_MAX_LCORE];
struct lcore_params {
uint16_t port_id;
uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t lcore_id;
} __rte_cache_aligned;
static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
@@ -205,9 +205,9 @@ check_worker_model_params(void)
static int
check_lcore_params(void)
{
- uint8_t queue, lcore;
+ uint8_t queue;
int socketid;
- uint16_t i;
+ uint16_t i, lcore;
for (i = 0; i < nb_lcore_params; ++i) {
queue = lcore_params[i].queue_id;
@@ -282,7 +282,7 @@ static int
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint16_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -452,7 +452,7 @@ parse_config(const char *q_arg)
lcore_params_array[nb_lcore_params].queue_id =
(uint8_t)int_fld[FLD_QUEUE];
lcore_params_array[nb_lcore_params].lcore_id =
- (uint8_t)int_fld[FLD_LCORE];
+ (uint16_t)int_fld[FLD_LCORE];
++nb_lcore_params;
}
lcore_params = lcore_params_array;
--
2.25.1
[PATCH 5/6] examples/qos_sched: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: de3cfa2c9823 ("sched: initial import")
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/qos_sched/args.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
index e97273152a..22fe76eeb5 100644
--- a/examples/qos_sched/args.c
+++ b/examples/qos_sched/args.c
@@ -182,10 +182,10 @@ app_parse_flow_conf(const char *conf_str)
pconf->rx_port = vals[0];
pconf->tx_port = vals[1];
- pconf->rx_core = (uint8_t)vals[2];
- pconf->wt_core = (uint8_t)vals[3];
+ pconf->rx_core = (uint16_t)vals[2];
+ pconf->wt_core = (uint16_t)vals[3];
if (ret == 5)
- pconf->tx_core = (uint8_t)vals[4];
+ pconf->tx_core = (uint16_t)vals[4];
else
pconf->tx_core = pconf->wt_core;
--
2.25.1
[PATCH 4/6] examples/ipsec-secgw: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
Cc: sergio.gonzalez.mon...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/ipsec-secgw/event_helper.h | 2 +-
examples/ipsec-secgw/ipsec-secgw.c | 12 +++-
examples/ipsec-secgw/ipsec.c| 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/examples/ipsec-secgw/event_helper.h
b/examples/ipsec-secgw/event_helper.h
index dfb81bfcf1..9923700f03 100644
--- a/examples/ipsec-secgw/event_helper.h
+++ b/examples/ipsec-secgw/event_helper.h
@@ -102,7 +102,7 @@ struct eh_event_link_info {
/**< Event port ID */
uint8_t eventq_id;
/**< Event queue to be linked to the port */
- uint8_t lcore_id;
+ uint16_t lcore_id;
/**< Lcore to be polling on this port */
};
diff --git a/examples/ipsec-secgw/ipsec-secgw.c
b/examples/ipsec-secgw/ipsec-secgw.c
index bf98d2618b..0c15ec5334 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -221,7 +221,7 @@ static const char *cfgfile;
struct lcore_params {
uint16_t port_id;
uint8_t queue_id;
- uint8_t lcore_id;
+ uint16_t lcore_id;
} __rte_cache_aligned;
static struct lcore_params lcore_params_array[MAX_LCORE_PARAMS];
@@ -810,7 +810,7 @@ check_flow_params(uint16_t fdir_portid, uint8_t fdir_qid)
static int32_t
check_poll_mode_params(struct eh_conf *eh_conf)
{
- uint8_t lcore;
+ uint16_t lcore;
uint16_t portid;
uint16_t i;
int32_t socket_id;
@@ -870,7 +870,7 @@ static int32_t
init_lcore_rx_queues(void)
{
uint16_t i, nb_rx_queue;
- uint8_t lcore;
+ uint16_t lcore;
for (i = 0; i < nb_lcore_params; ++i) {
lcore = lcore_params[i].lcore_id;
@@ -1051,6 +1051,8 @@ parse_config(const char *q_arg)
char *str_fld[_NUM_FLD];
int32_t i;
uint32_t size;
+ unsigned int max_fld[_NUM_FLD] = {RTE_MAX_ETHPORTS,
+ 255, RTE_MAX_LCORE};
nb_lcore_params = 0;
@@ -1071,7 +1073,7 @@ parse_config(const char *q_arg)
for (i = 0; i < _NUM_FLD; i++) {
errno = 0;
int_fld[i] = strtoul(str_fld[i], &end, 0);
- if (errno != 0 || end == str_fld[i] || int_fld[i] > 255)
+ if (errno != 0 || end == str_fld[i] || int_fld[i] >
max_fld[i])
return -1;
}
if (nb_lcore_params >= MAX_LCORE_PARAMS) {
@@ -1084,7 +1086,7 @@ parse_config(const char *q_arg)
lcore_params_array[nb_lcore_params].queue_id =
(uint8_t)int_fld[FLD_QUEUE];
lcore_params_array[nb_lcore_params].lcore_id =
- (uint8_t)int_fld[FLD_LCORE];
+ (uint16_t)int_fld[FLD_LCORE];
++nb_lcore_params;
}
lcore_params = lcore_params_array;
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index f5cec4a928..5ebb71bb9a 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -259,7 +259,7 @@ create_lookaside_session(struct ipsec_ctx
*ipsec_ctx_lcore[],
continue;
/* Looking for cryptodev, which can handle this SA */
- key.lcore_id = (uint8_t)lcore_id;
+ key.lcore_id = (uint16_t)lcore_id;
key.cipher_algo = (uint8_t)sa->cipher_algo;
key.auth_algo = (uint8_t)sa->auth_algo;
key.aead_algo = (uint8_t)sa->aead_algo;
--
2.25.1
[PATCH 6/6] examples/vm_power_manager: fix lcore ID restriction
Currently the config option allows lcore IDs up to 255,
irrespective of RTE_MAX_LCORES and needs to be fixed.
The patch allows config options based on DPDK config.
Fixes: 0e8f47491f09 ("examples/vm_power: add command to query CPU frequency")
Cc: marcinx.hajkow...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Sivaprasad Tummala
---
examples/vm_power_manager/guest_cli/vm_power_cli_guest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 94bfbbaf78..a586853a76 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -401,7 +401,7 @@ check_response_cmd(unsigned int lcore_id, int *result)
struct cmd_set_cpu_freq_result {
cmdline_fixed_string_t set_cpu_freq;
- uint8_t lcore_id;
+ uint16_t lcore_id;
cmdline_fixed_string_t cmd;
};
@@ -444,7 +444,7 @@ cmdline_parse_token_string_t cmd_set_cpu_freq =
set_cpu_freq, "set_cpu_freq");
cmdline_parse_token_num_t cmd_set_cpu_freq_core_num =
TOKEN_NUM_INITIALIZER(struct cmd_set_cpu_freq_result,
- lcore_id, RTE_UINT8);
+ lcore_id, RTE_UINT16);
cmdline_parse_token_string_t cmd_set_cpu_freq_cmd_cmd =
TOKEN_STRING_INITIALIZER(struct cmd_set_cpu_freq_result,
cmd, "up#down#min#max#enable_turbo#disable_turbo");
--
2.25.1
