[next-queue,2/2] ixgbe: add unlikely notes to tx fastpath expressions

Message ID 1515451669-927-2-git-send-email-shannon.nelson@oracle.com
State Accepted
Delegated to: Jeff Kirsher
Headers show
Series
  • Untitled series #22016
Related show

Commit Message

Shannon Nelson Jan. 8, 2018, 10:47 p.m.
Add unlikely() to a few error checking expressions in the Tx
offload handling.

Suggested-by: Yanjun Zhu <yanjun.zhu@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Bowers, AndrewX Jan. 10, 2018, 12:08 a.m. | #1
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Shannon Nelson
> Sent: Monday, January 8, 2018 2:48 PM
> To: intel-wired-lan@lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Cc: steffen.klassert@secunet.com; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH next-queue 2/2] ixgbe: add unlikely notes
> to tx fastpath expressions
> 
> Add unlikely() to a few error checking expressions in the Tx offload handling.
> 
> Suggested-by: Yanjun Zhu <yanjun.zhu@oracle.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Zhu Yanjun Jan. 18, 2018, 9:06 a.m. | #2
On 2018/1/9 6:47, Shannon Nelson wrote:
> Add unlikely() to a few error checking expressions in the Tx
> offload handling.
>
> Suggested-by: Yanjun Zhu <yanjun.zhu@oracle.com>
Hi,

I am fine with this patch. I have a question. The ipsec feature is 
supported in ixgbevf?

Thanks a lot.
Zhu Yanjun
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 57c10e6..3d069a2 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -749,28 +749,28 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
>   	struct xfrm_state *xs;
>   	struct tx_sa *tsa;
>   
> -	if (!first->skb->sp->len) {
> +	if (unlikely(!first->skb->sp->len)) {
>   		netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
>   			   __func__, first->skb->sp->len);
>   		return 0;
>   	}
>   
>   	xs = xfrm_input_state(first->skb);
> -	if (!xs) {
> +	if (unlikely(!xs)) {
>   		netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
>   			   __func__, xs);
>   		return 0;
>   	}
>   
>   	itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
> -	if (itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
> +	if (unlikely(itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
>   		netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
>   			   __func__, itd->sa_idx, xs->xso.offload_handle);
>   		return 0;
>   	}
>   
>   	tsa = &ipsec->tx_tbl[itd->sa_idx];
> -	if (!tsa->used) {
> +	if (unlikely(!tsa->used)) {
>   		netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
>   			   __func__, itd->sa_idx);
>   		return 0;
Shannon Nelson Jan. 18, 2018, 5:10 p.m. | #3
On 1/18/2018 1:06 AM, Yanjun Zhu wrote:
> On 2018/1/9 6:47, Shannon Nelson wrote:
>> Add unlikely() to a few error checking expressions in the Tx
>> offload handling.
>>
>> Suggested-by: Yanjun Zhu <yanjun.zhu@oracle.com>
> Hi,
> 
> I am fine with this patch. I have a question. The ipsec feature is 
> supported in ixgbevf?

The x540 datasheet doesn't show any IPsec registers in the VF space, so 
I'm pretty sure the answer is 'no'.

One of the difficulties in providing this is how to manage the register 
space needed in the chip.  Either there's one big table that the PF and 
all the VFs need to somehow coordinate to share, or there are separate 
tables for each PF and VF, taking up a lot of chip space.  I suspect the 
original 82599 designers just weren't ready to take on this issue, nor 
was there enough customer pull in the VF space yet for such a thing.

sln

> 
> Thanks a lot.
> Zhu Yanjun

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 57c10e6..3d069a2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -749,28 +749,28 @@  int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
 	struct xfrm_state *xs;
 	struct tx_sa *tsa;
 
-	if (!first->skb->sp->len) {
+	if (unlikely(!first->skb->sp->len)) {
 		netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
 			   __func__, first->skb->sp->len);
 		return 0;
 	}
 
 	xs = xfrm_input_state(first->skb);
-	if (!xs) {
+	if (unlikely(!xs)) {
 		netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
 			   __func__, xs);
 		return 0;
 	}
 
 	itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
-	if (itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
+	if (unlikely(itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
 		netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
 			   __func__, itd->sa_idx, xs->xso.offload_handle);
 		return 0;
 	}
 
 	tsa = &ipsec->tx_tbl[itd->sa_idx];
-	if (!tsa->used) {
+	if (unlikely(!tsa->used)) {
 		netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
 			   __func__, itd->sa_idx);
 		return 0;