mbuf: Improved error message. Added check if shared memory already allocated
PATCH 1/1 mbuf: Improved error message. Added check if shared memory already allocated
--- lib/mbuf/rte_mbuf_dyn.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
lib/mbuf/rte_mbuf_dyn.c +3 -2
@@ -116,7 +116,8 @@ init_shared_mem(void) mz =rte_memzone_lookup (RTE_MBUF_DYN_MZNAME ); }if (mz ==NULL ) { -RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory\n" ); +RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory: %s (%d)\n" , +rte_strerror (rte_errno), rte_errno);return -1 ; }
@@ -531,7 +532,7 @@ void rte_mbuf_dyn_dump(FILE *out)size_t i;rte_mcfg_tailq_write_lock (); -if (init_shared_mem () <0 ) { +if (shm ==NULL &&init_shared_mem () <0 ) {rte_mcfg_tailq_write_unlock ();return ; }
Signature · 1 line
2.30.2
Hi Alexander,
Thanks for submitting this patch. Few comments below.
Quoted 1 line · Alexander Bechikov
mbuf: Improved error message. Added check if shared memory already allocated
I suggest another title:
mbuf: fix dump of dynamic fields and flags
Indeed, it appears that the dump is currently broken. We can see it in the unit tests:
RTE>>mbuf_autotest
...
MBUF: Failed to get mbuf dyn shared memory
As the function returns void, we failure is not detected by the test.
Can you add a small commit log?
I suppose this issue is introduced by d4902ed31c63 ("mbuf: check shared memory before dumping dynamic space"). Do you confirm?
Can you please also add:
Quoted 10 lines
Signed-off-by Alexander Bechikov <[email protected]>--- lib/mbuf/rte_mbuf_dyn.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)lib/mbuf/rte_mbuf_dyn.c +0
-0
lib/mbuf/rte_mbuf_dyn.c init_shared_mem(void) · line 123
Quoted 9 lines
lib/mbuf/rte_mbuf_dyn.c +2
-1@@ -116,7 +116,8 @@ init_shared_mem(void) mz =rte_memzone_lookup (RTE_MBUF_DYN_MZNAME ); }if (mz ==NULL ) { -RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory\n" ); +RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory: %s (%d)\n" , +rte_strerror (rte_errno), rte_errno);return -1 ; }
I think rte_errno is not set when rte_memzone_lookup() returns NULL (this happens when init_shared_mem() is called from a secondary process). You can set it in this function to ENOENT, or just drop this change.
Quoted 12 lines
lib/mbuf/rte_mbuf_dyn.c +1
-1@@ -531,7 +532,7 @@ void rte_mbuf_dyn_dump(FILE *out)size_t i;rte_mcfg_tailq_write_lock (); -if (init_shared_mem () <0 ) { +if (shm ==NULL &&init_shared_mem () <0 ) {rte_mcfg_tailq_write_unlock ();return ; }Signature · 1 line
2.30.2
Thanks,
Olivier
PATCH v2 1/2 mbuf: Improved error message. Added check if shared memory already allocated
In reply to Alexander Bechikov--- lib/mbuf/rte_mbuf_dyn.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
lib/mbuf/rte_mbuf_dyn.c +3 -2
@@ -116,7 +116,8 @@ init_shared_mem(void) mz =rte_memzone_lookup (RTE_MBUF_DYN_MZNAME ); }if (mz ==NULL ) { -RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory\n" ); +RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory: %s (%d)\n" , +rte_strerror (rte_errno), rte_errno);return -1 ; }
@@ -531,7 +532,7 @@ void rte_mbuf_dyn_dump(FILE *out)size_t i;rte_mcfg_tailq_write_lock (); -if (init_shared_mem () <0 ) { +if (shm ==NULL &&init_shared_mem () <0 ) {rte_mcfg_tailq_write_unlock ();return ; }
Signature · 1 line
2.30.2
PATCH v2 2/2 mbuf: fix dump of dynamic fields and flags
I suppose this issue is introduced by d4902ed31c63 ("mbuf: check shared memory before dumping dynamic space").
---
v2:
* Drop changes with log message from v1, because rte_errno is not always set
--- lib/mbuf/rte_mbuf_dyn.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
lib/mbuf/rte_mbuf_dyn.c +1 -2
@@ -116,8 +116,7 @@ init_shared_mem(void) mz =rte_memzone_lookup (RTE_MBUF_DYN_MZNAME ); }if (mz ==NULL ) { -RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory: %s (%d)\n" , -rte_strerror (rte_errno), rte_errno); +RTE_LOG (ERR ,MBUF ,"Failed to get mbuf dyn shared memory\n" );return -1 ; }
Signature · 1 line
2.30.2
PATCH v3 1/1 mbuf: fix dump of dynamic fields and flags
In reply to Alexander BechikovFrom: Alexander Bechikov <[email protected]>
The dump of dynamic fields and flags fails if the shm is already allocated. Add a check to fix the issue.
---
Hi Alexander,
Sorry for late feedback, I forgot to reply to your v2. I'm resending your fix with the 2 patches merged together, so it can be included in 21.11.
Thanks,
Olivier
v3:
* Merge the 2 patches from v2 and add a commit log
v2:
* Drop changes with log message from v1, because rte_errno is not always set
--- lib/mbuf/rte_mbuf_dyn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/mbuf/rte_mbuf_dyn.c +1 -1
@@ -531,7 +531,7 @@ void rte_mbuf_dyn_dump(FILE *out)size_t i;rte_mcfg_tailq_write_lock (); -if (init_shared_mem () <0 ) { +if (shm ==NULL &&init_shared_mem () <0 ) {rte_mcfg_tailq_write_unlock ();return ; }
Signature · 1 line
2.30.2
Re: [PATCH v3] mbuf: fix dump of dynamic fields and flags
24/11/2021 13:57, Olivier Matz:
Quoted 8 lines
From: Alexander Bechikov <[email protected]>
The dump of dynamic fields and flags fails if the shm is already allocated. Add a check to fix the issue.
Fixes d4902ed31c63 ("mbuf: check shared memory before dumping dynamic space")
Quoted 8 lines
Signed-off-by Alexander Bechikov <[email protected]>Acked-by Olivier Matz <[email protected]>---
Hi Alexander,Sorry for late feedback, I forgot to reply to your v2. I'm resending your fix with the 2 patches merged together, so it can be included in 21.11.
Applied for 21.11, thanks.