From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f51.google.com (mail-wm0-f51.google.com [74.125.82.51]) by dpdk.org (Postfix) with ESMTP id 6BCA22B9F for ; Tue, 8 Mar 2016 23:53:44 +0100 (CET) Received: by mail-wm0-f51.google.com with SMTP id l68so47701453wml.1 for ; Tue, 08 Mar 2016 14:53:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:organization:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=d1ZQYUn0ACt3Ry+P1zFOJl8aURogk7epLk8lDZo5iyQ=; b=BhcKEvw+vDYRglxEm80bq/vFuS+pJGCohffclhlcD8OvzqesHxKmWro4/pS21nu80F NXq/HgU1ezmUHhQ5UJq2iKN5DB0MMJSbKb8i4YVH0df2SbfLzeFOh+E2R1bymR6JRn/n ifzbSfZ24Cw7w3uQ/NHZXqSlIInjfDII83Pw6juHYK8oWE/gFDyhf7bOmvDA/pRa/U0r +S5SpVXTDgJsm3e6lMi4rcT4SLo+PeKEK1oEZV+KqEBoB0uiAKRpYbFd4hezpsdsbQMc K4q9ITon+4iPVC+aZXMC1hKCV0opj1braMWnoev+F2l2qEuBzge2k7r9NpsHaWvriFdE wQyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding; bh=d1ZQYUn0ACt3Ry+P1zFOJl8aURogk7epLk8lDZo5iyQ=; b=lWKcyo4eUIzpLYqehPQpmPGHpEzKFnciYKb0R5l6OS8Cpg1yG0+MzJAvNF66iBC9WE QqrI5vxRXr4bWzW0Hn705nUM/dIu+YZ3+cKtJN0J1OMtIPsAqh6Tb9h9DuAavJqvFg15 XSI5LpsEyLyz7qL/hsIJzmuDzoqeirbTQLEZqp3CizsYFpkW6cRZfdlv9WSL6C7ksP5N br/hwLa2vocYh042slk4DaFScQNOUSbKbP3xzFR0JQZjLGlqSKXA0NIoDAsQAcs60zZM vGD3o2YbP0ys0jxbglO2O0DilVCIHrlhi5U/boQikN3+PaJ0B0/EfOGdR66+B4aUSf2C gs7g== X-Gm-Message-State: AD7BkJLeRe8dY8aPTTnphVNcflbNs401Y2qUdTReP7zq8/A5VFFh2ViUlKeA5b28hchFUK8W X-Received: by 10.28.145.8 with SMTP id t8mr22311602wmd.103.1457477624236; Tue, 08 Mar 2016 14:53:44 -0800 (PST) Received: from xps13.localnet (171.36.101.84.rev.sfr.net. [84.101.36.171]) by smtp.gmail.com with ESMTPSA id x186sm20895876wmg.19.2016.03.08.14.53.42 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 08 Mar 2016 14:53:43 -0800 (PST) From: Thomas Monjalon To: Tomasz Kulasek Date: Tue, 08 Mar 2016 23:52:05 +0100 Message-ID: <4446137.g0qt5Fe5Df@xps13> Organization: 6WIND User-Agent: KMail/4.14.10 (Linux/4.1.6-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <1456333729-3804-2-git-send-email-tomaszx.kulasek@intel.com> References: <1452869038-9140-1-git-send-email-tomaszx.kulasek@intel.com> <1456333729-3804-1-git-send-email-tomaszx.kulasek@intel.com> <1456333729-3804-2-git-send-email-tomaszx.kulasek@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add buffered tx api 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: Tue, 08 Mar 2016 22:53:44 -0000 Hi, It is an overlay on the tx burst API. Probably it doesn't hurt to add it but we have to be really cautious with the API definition to try keeping it stable in the future. 2016-02-24 18:08, Tomasz Kulasek: > +/** > + * Structure used to buffer packets for future TX > + * Used by APIs rte_eth_tx_buffer and rte_eth_tx_buffer_flush > + */ > +struct rte_eth_dev_tx_buffer { > + unsigned nb_pkts; What about "length"? Why is it unsigned and the size is uint16_t? > + uint64_t errors; > + /**< Total number of queue packets to sent that are dropped. */ The errors are passed as userdata to the default callback. If we really want to have this kind of counter, we can define our own callback. So why defining this field as standard? I would like to keep it as simple as possible. > + buffer_tx_error_fn cbfn; Why not simply "callback" as name? > + void *userdata; > + uint16_t size; /**< Size of buffer for buffered tx */ > + struct rte_mbuf *pkts[]; > +}; What is the benefit of exposing this structure in the API, except that it is used in some inline functions? > +static inline uint16_t > +rte_eth_tx_buffer_flush(uint8_t port_id, uint16_t queue_id, > + struct rte_eth_dev_tx_buffer *buffer) > +{ > + uint16_t sent; > + > + uint16_t to_send = buffer->nb_pkts; > + > + if (to_send == 0) > + return 0; Why this check is done in the lib? What is the performance gain if we are idle? It can be done outside if needed. > + sent = rte_eth_tx_burst(port_id, queue_id, buffer->pkts, to_send); > + > + buffer->nb_pkts = 0; > + > + /* All packets sent, or to be dealt with by callback below */ > + if (unlikely(sent != to_send)) > + buffer->cbfn(&buffer->pkts[sent], to_send - sent, > + buffer->userdata); > + > + return sent; > +} [...] > +/** > + * Callback function for tracking unsent buffered packets. > + * > + * This function can be passed to rte_eth_tx_buffer_set_err_callback() to > + * adjust the default behaviour when buffered packets cannot be sent. This > + * function drops any unsent packets, but also updates a user-supplied counter > + * to track the overall number of packets dropped. The counter should be an > + * uint64_t variable. > + * > + * NOTE: this function should not be called directly, instead it should be used > + * as a callback for packet buffering. > + * > + * NOTE: when configuring this function as a callback with > + * rte_eth_tx_buffer_set_err_callback(), the final, userdata parameter > + * should point to an uint64_t value. Please forget this idea of counter in the default callback. [...] > +void > +rte_eth_count_unsent_packet_callback(struct rte_mbuf **pkts, uint16_t unsent, > + void *userdata); What about rte_eth_tx_buffer_default_callback as name?