From: Jakub Kicinski <kuba@kernel.org>
To: Guillaume Nault <gnault@redhat.com>
Cc: David Miller <davem@davemloft.net>,
Paolo Abeni <pabeni@redhat.com>,
Eric Dumazet <edumazet@google.com>,
netdev@vger.kernel.org, Simon Horman <horms@kernel.org>,
David Ahern <dsahern@kernel.org>,
Andrew Lunn <andrew+netdev@lunn.ch>
Subject: Re: [PATCH net-next 1/4] vrf: Make pcpu_dstats update functions available to other modules.
Date: Mon, 2 Dec 2024 19:59:24 -0800 [thread overview]
Message-ID: <20241202195924.30affd25@kernel.org> (raw)
In-Reply-To: <5e97f1e54e57b0a85e34af87062dc536a28bef34.1733175419.git.gnault@redhat.com>
On Mon, 2 Dec 2024 22:48:48 +0100 Guillaume Nault wrote:
> - int len = skb->len;
> netdev_tx_t ret = is_ip_tx_frame(skb, dev);
> + unsigned int len = skb->len;
You can't reorder skb->len init after is_ip_tx_frame().
IDK what is_ stands for but that function xmits / frees the skb.
You're already making this code cleaner, let's take another step
forward and move that call out of line. Complex functions should not
be called as part of variable init IMHO. It makes the code harder to
read at best and error prone at worst..
--
pw-bot: cr
next prev parent reply other threads:[~2024-12-03 3:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-02 21:48 [PATCH net-next 0/4] net: Convert some UDP tunnel drivers to NETDEV_PCPU_STAT_DSTATS Guillaume Nault
2024-12-02 21:48 ` [PATCH net-next 1/4] vrf: Make pcpu_dstats update functions available to other modules Guillaume Nault
2024-12-03 3:59 ` Jakub Kicinski [this message]
2024-12-03 11:32 ` Guillaume Nault
2024-12-02 21:48 ` [PATCH net-next 2/4] vxlan: Handle stats using NETDEV_PCPU_STAT_DSTATS Guillaume Nault
2024-12-02 21:48 ` [PATCH net-next 3/4] geneve: " Guillaume Nault
2024-12-02 21:48 ` [PATCH net-next 4/4] bareudp: " Guillaume Nault
2024-12-03 8:12 ` [PATCH net-next 0/4] net: Convert some UDP tunnel drivers to NETDEV_PCPU_STAT_DSTATS James Chapman
2024-12-03 11:39 ` Guillaume Nault
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=20241202195924.30affd25@kernel.org \
--to=kuba@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=gnault@redhat.com \
--cc=horms@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.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).