netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@intel.com>
Cc: "intel-wired-lan@osuosl.org" <intel-wired-lan@osuosl.org>,
	"Gomes, Vinicius" <vinicius.gomes@intel.com>,
	"naamax.meir@linux.intel.com" <naamax.meir@linux.intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"leon@kernel.org" <leon@kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"edumazet@google.com" <edumazet@google.com>,
	"tee.min.tan@linux.intel.com" <tee.min.tan@linux.intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Neftin, Sasha" <sasha.neftin@intel.com>
Subject: Re: [PATCH net-next v3] igc: offload queue max SDU from tc-taprio
Date: Mon, 13 Feb 2023 22:40:03 -0800	[thread overview]
Message-ID: <20230213224003.1af75612@kernel.org> (raw)
In-Reply-To: <SJ1PR11MB61801B3439A4F19C32ADE11BB8A29@SJ1PR11MB6180.namprd11.prod.outlook.com>

On Tue, 14 Feb 2023 06:27:17 +0000 Zulkifli, Muhammad Husaini wrote:
> > > +	if (tx_ring->max_sdu > 0) {
> > > +		max_sdu = tx_ring->max_sdu +
> > > +			  (skb_vlan_tagged(skb) ? VLAN_HLEN : 0);
> > > +
> > > +		if (skb->len > max_sdu)  
> > 
> > You should increment some counter here. Otherwise it's a silent discard.  
> 
> I am thinking to use tx_dropped counters for this. Is it ok?

Yup!

> > > +			goto skb_drop;
> > > +	}
> > > +
> > >  	if (!tx_ring->launchtime_enable)
> > >  		goto done;
> > >
> > > @@ -1606,6 +1615,11 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,  
> > >  	dev_kfree_skb_any(first->skb);  
> > 
> > first->skb is skb, as far as I can tell, you can reshuffle this code to
> > avoid adding the new return flow.  
> 
> What we try to do is to check the current max_sdu size at the
> beginning stage of the func() and drop it quickly.

I understand, what I'm saying is that the code which is already here
can be reused, it currently operates on first->skb but it can as well
use skb. And then you'll be able to jump to the same statement, rather
than have to create a separate return.

  reply	other threads:[~2023-02-14  6:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-08  0:33 [PATCH net-next v3] igc: offload queue max SDU from tc-taprio Muhammad Husaini Zulkifli
2023-02-09  5:30 ` Jakub Kicinski
2023-02-14  6:27   ` Zulkifli, Muhammad Husaini
2023-02-14  6:40     ` Jakub Kicinski [this message]
2023-02-14  6:49       ` Zulkifli, Muhammad Husaini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230213224003.1af75612@kernel.org \
    --to=kuba@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@osuosl.org \
    --cc=leon@kernel.org \
    --cc=muhammad.husaini.zulkifli@intel.com \
    --cc=naamax.meir@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sasha.neftin@intel.com \
    --cc=tee.min.tan@linux.intel.com \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).