Re: [dpdk-dev] [PATCH] rte_sched: correctly free allocated subport memory
On 27. 05. 2020. 13:48, Singh, Jasvinder wrote:
>
>> -Original Message-
>> From: dev On Behalf Of Hrvoje Habjanic
>> Sent: Tuesday, May 26, 2020 6:25 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] rte_sched: correctly free allocated subport
>> memory
>>
>> In function rte_sched_subport_free (lib/librte_sched/rte_sched.c, line 865),
>> there is code to free all allocated stuff related to scheduler subport. First
>> there are some checks, and in the end, rte_bitmap_free is called.
>>
>> Now, rte_bitmap_free is a dummy function, and it just checks if provided
>> pointer to bitmap is valid or not. So, actual memory for subport is not
>> freed.
>>
>> This patch fixes this by removing call to rte_bitmap_free, and instead
>> calling
>> rte_free.
>>
>> Signed-off-by: Hrvoje Habjanic
>> ---
>> lib/librte_sched/rte_sched.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
>> index
>> c0983ddda..f15a3b515 100644
>> --- a/lib/librte_sched/rte_sched.c
>> +++ b/lib/librte_sched/rte_sched.c
>> @@ -888,7 +888,7 @@ rte_sched_subport_free(struct rte_sched_port *port,
>> }
>> }
>>
>> -rte_bitmap_free(subport->bmp);
>> +rte_free(subport);
>> }
>>
>> void
>> --
>> 2.17.1
> Hi Hrvoje;
>
> I guess this is your first patch to dpdk.org, here are some suggestions when
> you send bug fixes;
Yes, it is.
>
> - When sending fixes, please use "fix" word in the subject line, e.g-
> rte_sched: fix subport memory leak
> - The commit message should include commit id corresponding to the line that
> you fixes as shown below for this case.
>Fixes: d9213b829a31 ("sched: remove pipe params config from port level")
OK, noted, thank you.
> Patch looks good to me.
Great.
As s side note, it would be nice if this could be backported to 19.11 LTS.
Regards,
H.
>
> Acked-by: Jasvinder Singh
>
>
[dpdk-dev] [DPDK API question]
ss
Hi, Guys
I am learning data structure defined by dpdk framework and I noticed
a data structure definition below:
enum rte_eth_hash_function {
RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
/**
* Symmetric Toeplitz: src, dst will be replaced by
* xor(src, dst). For the case with src/dst only,
* src or dst address will xor with zero pair.
*/
RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
RTE_ETH_HASH_FUNCTION_MAX,
};
I have a little difficulty understanding that for the comment:
/**
* Symmetric Toeplitz: src, dst will be replaced by
* xor(src, dst). For the case with src/dst only,
* src or dst address will xor with zero pair.
*/
if user configure rss func for symmetric toeplitz, Rss type must be
updated at the same time?
if user configure rss func for xor, rss type must be empty and the queue
number is zero?
Must it be restricted like this? If so, what is his benefit?
Looking forward to your reply
Thanks
Lijun Ou
Re: [dpdk-dev] [EXT] Re: [PATCH 3/3] l3fwd-power: add interrupt-only mode
On Fri, May 29, 2020 at 03:19:45PM +0100, Burakov, Anatoly wrote: > External Email > > -- > On 29-May-20 2:19 PM, Harman Kalra wrote: > > > > if (ret < 0) > > > rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n"); > > > - if (app_mode != APP_MODE_TELEMETRY && init_power_library()) > > > + if (app_mode == APP_MODE_DEFAULT) > > > + app_mode = APP_MODE_LEGACY; > > > + > > > + /* only legacy and empty poll mode rely on power library */ > > > + if ((app_mode == APP_MODE_LEGACY || app_mode == APP_MODE_EMPTY_POLL) && > > > + init_power_library()) > > > rte_exit(EXIT_FAILURE, "init_power_library failed\n"); > > Hi, > > > > Rather than just exiting from here can we have a else condition to > > automatically enter into the "interrupt only" mode. > > Please correct me if I am missing something. > > Hi, > > Thanks for your review. I don't think silently proceeding is a good idea. If > the user wants interrupt-only mode, they should request it. Silently falling > back to interrupt-only mode will create an illusion of successful > initialization and set the wrong expectation for how the application will > behave. > Hi, Thanks for the explanation which even I also believe is logically perfect. But since l3fwd-power is an old application and has many users around which are currently using this app in interrupt only mode without giving an extra argument. But suddenly they will start getting failure messages with the new patchset. My only intent with else condition was backward compatibility. Or may be we can have more descriptive failure message, something like "init_power_library failed, check manual for other modes". Thanks Harman > -- > Thanks, > Anatoly
Re: [dpdk-dev] [EXT] Re: [PATCH 1/3] mbuf: add Tx offloads for packet marking
> > > I also share Olivier's concern about consuming 3 bits in ol_flags for
> > > that feature.
> > > Can it probably be squeezed somehow?
> > > Let say we reserve one flag that this information is present or not, and
> > > re-use one of rx-only fields for store additional information
> > > (packet_type, or so).
> > > Or might be some other approach.
> >
> > We are fine with this approach where we define one bit in Tx offloads for
> > pkt
> > marking and and 3 bits reused from Rx offload flags area.
> >
> > For example:
> >
> > @@ -186,10 +186,16 @@ extern "C" {
> >
> > /* add new RX flags here, don't forget to update PKT_FIRST_FREE */
> >
> > +/* Reused Rx offload bits for Tx offloads */
> > +#define PKT_X_TX_MARK_VLAN_DEI (1ULL << 0)
> > +#define PKT_X_TX_MARK_IP_DSCP (1ULL << 1)
> > +#define PKT_X_TX_MARK_IP_ECN (1ULL << 2)
> > +
> > #define PKT_FIRST_FREE (1ULL << 23)
> > -#define PKT_LAST_FREE (1ULL << 40)
> > +#define PKT_LAST_FREE (1ULL << 39)
> >
> > /* add new TX flags here, don't forget to update PKT_LAST_FREE */
> > +#define PKT_TX_MARK_EN (1ULL << 40)
> >
> > Is this fine ?
>
> Any thoughts on this approach which uses only 1 bit in Tx flags out of 18
> and reuse unused Rx flag bits ?
+ Techboard
There is a related thread going on
http://mails.dpdk.org/archives/dev/2020-May/168810.html
If there is no consensus on email, then I would like to add this item
to the next TB meeting.
