From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DC44CA04A2 for ; Tue, 12 May 2020 12:58:00 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AAE9E1BFE1; Tue, 12 May 2020 12:58:00 +0200 (CEST) Received: from huawei.com (szxga03-in.huawei.com [45.249.212.189]) by dpdk.org (Postfix) with ESMTP id 2AF261BEE4; Tue, 12 May 2020 12:57:55 +0200 (CEST) Received: from DGGEMM401-HUB.china.huawei.com (unknown [172.30.72.57]) by Forcepoint Email with ESMTP id 6DE42EA78A4C56812C48; Tue, 12 May 2020 18:57:52 +0800 (CST) Received: from DGGEMM533-MBX.china.huawei.com ([169.254.5.111]) by DGGEMM401-HUB.china.huawei.com ([10.3.20.209]) with mapi id 14.03.0487.000; Tue, 12 May 2020 18:57:45 +0800 From: wangyunjian To: Gagandeep Singh , "dev@dpdk.org" CC: Hemant Agrawal , "Lilijun (Jerry)" , xudingke , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd Thread-Index: AQHWJ03ohEpJ9z9gjUGvXuoEchaPkaijS86AgAD1MjA= Date: Tue, 12 May 2020 10:57:44 +0000 Message-ID: <34EFBCA9F01B0748BEB6B629CE643AE60CFD82D5@DGGEMM533-MBX.china.huawei.com> References: <1589171838-18076-1-git-send-email-wangyunjian@huawei.com> In-Reply-To: Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.173.251.152] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" > -----Original Message----- > From: Gagandeep Singh [mailto:G.Singh@nxp.com] > Sent: Tuesday, May 12, 2020 11:49 AM > To: wangyunjian ; dev@dpdk.org > Cc: Hemant Agrawal ; Lilijun (Jerry) > ; xudingke ; > stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd >=20 >=20 >=20 > > -----Original Message----- > > From: wangyunjian > > Sent: Monday, May 11, 2020 10:07 AM > > To: dev@dpdk.org > > Cc: Gagandeep Singh ; Hemant Agrawal > > ; jerry.lilijun@huawei.com; > > xudingke@huawei.com; Yunjian Wang ; > > stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd > > > > From: Yunjian Wang > > > > Zero is a valid fd. It will fail to check the fd if the fd is zero. > > The "job_ring->uio_fd" is an fd, so define it as "int". > > > > Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations") > > Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang > > --- > > v2: > > * Change "job_ring->uio_fd" type suggested by Gagandeep Singh > > --- > > drivers/crypto/caam_jr/caam_jr.c | 6 ++++-- > > drivers/crypto/caam_jr/caam_jr_hw_specific.h | 2 +- > > drivers/crypto/caam_jr/caam_jr_pvt.h | 5 +++-- > > drivers/crypto/caam_jr/caam_jr_uio.c | 21 ++++++++++++++------ > > 4 files changed, 23 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/crypto/caam_jr/caam_jr.c > > b/drivers/crypto/caam_jr/caam_jr.c > > index 5a29dd169..f62d46546 100644 > > --- a/drivers/crypto/caam_jr/caam_jr.c > > +++ b/drivers/crypto/caam_jr/caam_jr.c > > @@ -2074,7 +2074,7 @@ static struct rte_security_ops > > caam_jr_security_ops =3D { static void close_job_ring(struct > > sec_job_ring_t *job_ring) { > > - if (job_ring->irq_fd) { > > + if (job_ring->irq_fd !=3D -1) { > > /* Producer index is frozen. If consumer index is not equal > > * with producer index, then we have descs to flush. > > */ > > @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev) > > * > > */ > > static void * > > -init_job_ring(void *reg_base_addr, uint32_t irq_id) > > +init_job_ring(void *reg_base_addr, int irq_id) > > { > > struct sec_job_ring_t *job_ring =3D NULL; > > int i, ret =3D 0; > > @@ -2466,6 +2466,8 @@ > > RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv, > > cryptodev_caam_jr_drv.driver, > > > > RTE_INIT(caam_jr_init_log) > > { > > + sec_init(); > > + > Why are you calling a fd initialization function in log registration cons= tructor? > This is not the right place for any initialization other than logs. > Also, sec_init function will not work here, as you are using "g_uio_jr_n= um" > whose value will always be 0 during this function call. Thanks, my bad. I should use 'MAX_SEC_JOB_RINGS'. The default values of 'g_job_rings' and 'g_uio_job_ring' are 0. As a result, the validity cannot be checked. what about following: void sec_uio_job_rings_init(void) { int i; for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) g_uio_job_ring[i].uio_fd =3D -1; } void sec_job_rings_init(void) { int i; for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) g_job_rings[i].irq_fd =3D -1; } RTE_INIT(caam_jr_init) { sec_uio_job_rings_init(); sec_job_rings_init(); } >=20 > > caam_jr_logtype =3D rte_log_register("pmd.crypto.caam"); > > if (caam_jr_logtype >=3D 0) > > rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); diff --git > > a/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > b/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > index 5f58a585d..ec4539d1b 100644 > > --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h > > @@ -360,7 +360,7 @@ struct sec_job_ring_t { > > * bitwise operations. > > */ > > > > - uint32_t irq_fd; /* The file descriptor used for polling from > > + int irq_fd; /* The file descriptor used for polling from > > * user space for interrupts notifications > > */ > > uint32_t jr_mode; /* Model used by SEC Driver to receive > > diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h > > b/drivers/crypto/caam_jr/caam_jr_pvt.h > > index 98cd4438a..fa9e86ea6 100644 > > --- a/drivers/crypto/caam_jr/caam_jr_pvt.h > > +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h > > @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len) } struct > > uio_job_ring { > > uint32_t jr_id; > > - uint32_t uio_fd; > > + int uio_fd; > > void *register_base_addr; > > int map_size; > > int uio_minor_number; > > }; > > > > int sec_cleanup(void); > > +void sec_init(void); > > int sec_configure(void); > > struct uio_job_ring *config_job_ring(void); -void > > free_job_ring(uint32_t uio_fd); > > +void free_job_ring(int uio_fd); > > > > /* For Dma memory allocation of specified length and alignment */ > > static inline void * diff --git a/drivers/crypto/caam_jr/caam_jr_uio.c > > b/drivers/crypto/caam_jr/caam_jr_uio.c > > index b1bb44ca4..133b32084 100644 > > --- a/drivers/crypto/caam_jr/caam_jr_uio.c > > +++ b/drivers/crypto/caam_jr/caam_jr_uio.c > > @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const char > > subdir[], > > "%s/%s/%s", root, subdir, filename); > > > > fd =3D open(absolute_file_name, O_RDONLY); > > - SEC_ASSERT(fd > 0, fd, "Error opening file %s", > > + SEC_ASSERT(fd >=3D 0, fd, "Error opening file %s", > > absolute_file_name); > > > > /* read UIO device name from first line in file */ @@ -322,7 +322,7 > > @@ uio_map_registers(int uio_device_fd, int uio_device_id, } > > > > void > > -free_job_ring(uint32_t uio_fd) > > +free_job_ring(int uio_fd) > > { > > struct uio_job_ring *job_ring =3D NULL; > > int i; >=20 > Please update the check "if (!uio_fd)". OK, I will fix it. >=20 > > @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd) > > job_ring->jr_id, job_ring->uio_fd); > > close(job_ring->uio_fd); > > g_uio_jr_num--; > > - job_ring->uio_fd =3D 0; > > + job_ring->uio_fd =3D -1; > > if (job_ring->register_base_addr =3D=3D NULL) > > return; > > > > @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void) > > int i; > > > > for (i =3D 0; i < MAX_SEC_JOB_RINGS; i++) { > > - if (g_uio_job_ring[i].uio_fd =3D=3D 0) { > > + if (g_uio_job_ring[i].uio_fd =3D=3D -1) { > > job_ring =3D &g_uio_job_ring[i]; > > g_uio_jr_num++; > > break; > > @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void) > > > > /* Open device file */ > > job_ring->uio_fd =3D open(uio_device_file_name, O_RDWR); > > - SEC_ASSERT(job_ring->uio_fd > 0, NULL, > > + SEC_ASSERT(job_ring->uio_fd >=3D 0, NULL, > > "Failed to open UIO device file for job ring %d", > > job_ring->jr_id); > > > > @@ -488,12 +488,21 @@ sec_cleanup(void) > > /* I need to close the fd after shutdown UIO commands need to be > > * sent using the fd > > */ > > - if (job_ring->uio_fd !=3D 0) { > > + if (job_ring->uio_fd !=3D -1) { > > CAAM_JR_INFO( > > "Closed device file for job ring %d , fd =3D %d", > > job_ring->jr_id, job_ring->uio_fd); > > close(job_ring->uio_fd); > > + job_ring->uio_fd =3D -1; > > } > > } > > return 0; > > } > > + > > +void sec_init(void) > > +{ > > + int i; > > + > > + for (i =3D 0; i < g_uio_jr_num; i++) > > + g_uio_job_ring[i].uio_fd =3D -1; > > +} > > -- > > 2.19.1 > > >=20 > There are still some functions(sec_uio_send_command, caam_jr_enable_irqs, > caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd parame= ter > as uint32_t, please update them also. >=20 I will update them in next version.