From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.rohde-schwarz.com (mail02.rohde-schwarz.com [80.246.32.97]) by dpdk.org (Postfix) with ESMTP id 3E0052C06 for ; Mon, 1 Aug 2016 10:18:26 +0200 (CEST) Received: from amu316.rsint.net ([10.0.26.65]) by mail02.rohde-schwarz.com with ESMTP id 2016080110180496-59416 ; Mon, 1 Aug 2016 10:18:04 +0200 Received: from rus19.rsint.net ([10.0.33.19]) by amu316.rsint.net (Totemo SMTP Server) with SMTP ID 802 for ; Mon, 1 Aug 2016 10:18:04 +0200 (CEST) To: dev@dpdk.org MIME-Version: 1.0 X-KeepSent: F02D03B3:A3BE6E7F-C1258002:002D2A13; type=4; flags=0; name=$KeepSent X-Mailer: IBM Notes Release 9.0.1FP6 April 21, 2016 From: Steffen.Bauch@rohde-schwarz.com X-RUS_SENSITIVITY: 10 Message-ID: Date: Mon, 1 Aug 2016 10:18:01 +0200 X-MIMETrack: Itemize by SMTP Server on RSSMTP02/RSSMTP at 01.08.2016 10:18:04, Serialize by Router on RSSMTP02/RSSMTP at 01.08.2016 10:18:25 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [dpdk-dev] Application framework vs. library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2016 08:18:26 -0000 Hi, as a developer of a product that uses DPDK I inherited a custom changeset towards DPDK 2.0 to allow better integration into our application stack. I am currently underway of updating the used DPDK release and would like to get rid of these local changes in the future by influencing the upstream development to some degree. Concerning the integration we try to cope with the circumstance that DPDK is more comparable to an application framework and less to a library. The main part of the changes deal with EAL and specifically with rte=5Feal=5Finit() and prune or replace functionality. The pruned features are in the responsibility of other proprietary libraries and modules within our application. For illustration purposes, I have included an old changeset towards v2.0.0. for reference, but due to the nature of the whole changeset it does not make sense to apply it directly, as there is a total lack of generality in the specific implementation (e.g. only the linuxapp part is customized, incomplete implemented concept in termination behavior ...). Nonetheless I am underway of researching concepts and approaches that are general enough for upstream inclusion. The main reason for attaching this patchset is to be able to discuss possible approaches of further development, as I would like to get feedback on possible development approaches to address the named problems. In detail the changes address the following areas: Logging behavior Opening a connection to the system logger for our program is within the responsibility of our specific logging module that is also used in the proprietary parts of the application. For this reason openlog() should not be called in our use case. In addition, using rte=5Fopenlog=5Fstream() is not usable before rte=5Feal=5Finit() as logging init will overwrite the used log stream. For this reason a large part of the init logs can not be handled by a custom log stream. Btw, after removal of the log caching, is there a fundamental difference between early log and normal logging? A possible approach for the future could be to initialize rte=5Flogs.file to NULL (in fact it is, as it is static) and only set it during initialization if it is NULL with the goal to be able to use rte=5Fopenlog=5Fstream() before rte=5Feal=5Finit(). As the openlog() call is only relevant when the default log stream is used (and architecture dependent!) I introduced a specific entry point for calling openlog. The main complexity to this changeset is added due to two reasons. First reason is the circumstance that rte=5Feal=5Fcommon=5Flog=5Finit() is used s= everal times during early logging and real logging initialization. Second aspect is that a user might revert to the use of the default=5Flog=5Fstream after a custom log stream has be used straight from the beginning (even before rte=5Feal=5Finit()). A dedicated changeset towards v16.07 for this improvement is attached for review, feedback and possible inclusion. Process termination behavior As the DPDK functionality is only a small part of the whole application ownership of the running process is not with DPDK and it is not allowed to cancel the running process as a result of an error. For this reason, the current changeset instead of calling rte=5Fpanic or rte=5Fexit returns an error code and handles it in the calling function. Unfortunately this change was only implemented in rte=5Feal=5Finit() and not in other places (drivers, libs ...), so there is currently no safety that an unknown code part terminates the whole application on the spot. Future changes and patches could change the error handling in a more thorough approach, but I would like to have some feedback and opinions before I start the heavy-lifting work in over 700 code locations. Even some interfaces might be affected, as lots of functions return void now, Thread creation In our application thread creation and setting the affinity is already handled in a proprietary module. Frames are polled and processed in non-EAL pthreads. For this reason the lcore thread creation in rte=5Feal=5Finit() is completely removed as we do not want additional threa= ds to be assigned to processing CPUs. Unfortunately setting the coremask to 0 is not feasible directly. For the non-EAL threads a custom function within the application sets the per=5Fthread =5Flcore variable to the right value to try to enable mbuf cacheing. The changeset also disables the interrupt handling thread as in our application it currently seems not necessary (maybe make this optional?) In an experiment I removed the error check for the non-zero number of lcores and quick-fixed certain locations where rte=5Flcore=5Fcount() is used for memory size computations and at least got a running application. A possible approach could be to enhance enum rte=5Flcore=5Frole=5Ft with an auxiliary thread type and introduce additional eal arguments to assign lcores with these auxiliary role. I would like to receive feedback on possible development directions. Thank you, Steffen Bauch --=20 Steffen Bauch Senior Software Engineer Platform & Technology R&S Cybersecurity ipoque GmbH Augustusplatz 9, D-04109 Leipzig Phone: + 49 341 59403 0 Email: steffen.bauch@rohde-schwarz.com Internet: www.ipoque.com Trade register Amtsgericht Leipzig HRB21462 Gesellschaft mit beschr=E4nkter Haftung (GmbH) Dirk Czepluch - Managing Director >From sagi@grimberg.me Mon Aug 1 10:44:25 2016 Return-Path: Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by dpdk.org (Postfix) with ESMTP id 99B3F2C0C for ; Mon, 1 Aug 2016 10:44:25 +0200 (CEST) Received: from bzq-82-81-101-184.red.bezeqint.net ([82.81.101.184] helo=bombadil.infradead.org) by merlin.infradead.org with esmtpsa (Exim 4.85_2 #1 (Red Hat Linux)) id 1bU8pk-00089b-B4 for dev@dpdk.org; Mon, 01 Aug 2016 08:44:24 +0000 From: Sagi Grimberg To: dev@dpdk.org Date: Mon, 1 Aug 2016 11:44:21 +0300 Message-Id: <1470041061-8059-1-git-send-email-sagi@grimberg.me> X-Mailer: git-send-email 1.9.1 Subject: [dpdk-dev] [PATCH] net/mlx5: Fix possible NULL deref in RX path X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Aug 2016 08:44:25 -0000 The user is allowed to call ->rx_pkt_burst() even without free mbufs in the pool. In this scenario we'll fail allocating a rep mbuf on the first iteration (where pkt is still NULL). This would cause us to deref a NULL pkt (reset refcount and free). Fix this by checking the pkt before freeing it. Signed-off-by: Sagi Grimberg --- drivers/net/mlx5/mlx5_rxtx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index fce3381ae87a..a07cc4794023 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1572,7 +1572,7 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) rte_prefetch0(wqe); rep = rte_mbuf_raw_alloc(rxq->mp); if (unlikely(rep == NULL)) { - while (pkt != seg) { + while (pkt && pkt != seg) { assert(pkt != (*rxq->elts)[idx]); seg = NEXT(pkt); rte_mbuf_refcnt_set(pkt, 0); -- 1.9.1