[net-next] ipvlan: minor optimization for ipvlan outbound process
Commit Message
Avoid some local variable initialization and remove some
redundant assignment in ipvlan outbound process.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
drivers/net/ipvlan/ipvlan_core.c | 56 +++++++++++++++-----------------
1 file changed, 26 insertions(+), 30 deletions(-)
Comments
On Tue, Nov 1, 2022 at 7:15 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> Avoid some local variable initialization and remove some
> redundant assignment in ipvlan outbound process.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Really I do not see the point of such a patch, making future backports
more difficult.
Changing old code like that should only be done if this is really necessary,
for instance before adding a new functionality.
On 2022/11/2 11:23, Eric Dumazet wrote:
> On Tue, Nov 1, 2022 at 7:15 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> Avoid some local variable initialization and remove some
>> redundant assignment in ipvlan outbound process.
>>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>
> Really I do not see the point of such a patch, making future backports
> more difficult.
As the ipvlan outbound process is in the fast path, avoiding the
unnecessary steps might be worth the backport cost.
Anyway, it is more of judgment call, not a rule, right?
>
> Changing old code like that should only be done if this is really necessary,
> for instance before adding a new functionality.
> .
>
On Tue, Nov 1, 2022 at 8:47 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2022/11/2 11:23, Eric Dumazet wrote:
> > On Tue, Nov 1, 2022 at 7:15 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>
> >> Avoid some local variable initialization and remove some
> >> redundant assignment in ipvlan outbound process.
> >>
> >> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> >
> > Really I do not see the point of such a patch, making future backports
> > more difficult.
>
> As the ipvlan outbound process is in the fast path, avoiding the
> unnecessary steps might be worth the backport cost.
Have you measured the gains after your patch is applied ?
Please give us some numbers, but I bet this will be pure noise,
given the overall ipvlan cost.
>
> Anyway, it is more of judgment call, not a rule, right?
The thing is, you are asking us maintainers/reviewers to spend time on
a non-trivial patch,
with no clear indication of why this is worth our time, and why this
is worth future merge conflicts in backports.
>
> >
> > Changing old code like that should only be done if this is really necessary,
> > for instance before adding a new functionality.
> > .
> >
@@ -417,7 +417,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
struct net_device *dev = skb->dev;
struct net *net = dev_net(dev);
struct rtable *rt;
- int err, ret = NET_XMIT_DROP;
+ int err;
struct flowi4 fl4 = {
.flowi4_oif = dev->ifindex,
.flowi4_tos = RT_TOS(ip4h->tos),
@@ -438,15 +438,14 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
skb_dst_set(skb, &rt->dst);
err = ip_local_out(net, skb->sk, skb);
if (unlikely(net_xmit_eval(err)))
- dev->stats.tx_errors++;
- else
- ret = NET_XMIT_SUCCESS;
- goto out;
+ goto tx_errors_inc;
+
+ return NET_XMIT_SUCCESS;
err:
- dev->stats.tx_errors++;
kfree_skb(skb);
-out:
- return ret;
+tx_errors_inc:
+ dev->stats.tx_errors++;
+ return NET_XMIT_DROP;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -456,7 +455,7 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
struct net_device *dev = skb->dev;
struct net *net = dev_net(dev);
struct dst_entry *dst;
- int err, ret = NET_XMIT_DROP;
+ int err;
struct flowi6 fl6 = {
.flowi6_oif = dev->ifindex,
.daddr = ip6h->daddr,
@@ -469,22 +468,23 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
dst = ip6_route_output(net, NULL, &fl6);
if (dst->error) {
- ret = dst->error;
+ err = dst->error;
dst_release(dst);
goto err;
}
skb_dst_set(skb, dst);
err = ip6_local_out(net, skb->sk, skb);
- if (unlikely(net_xmit_eval(err)))
- dev->stats.tx_errors++;
- else
- ret = NET_XMIT_SUCCESS;
- goto out;
+ if (unlikely(net_xmit_eval(err))) {
+ err = NET_XMIT_DROP;
+ goto tx_errors_inc;
+ }
+
+ return NET_XMIT_SUCCESS;
err:
- dev->stats.tx_errors++;
kfree_skb(skb);
-out:
- return ret;
+tx_errors_inc:
+ dev->stats.tx_errors++;
+ return err;
}
#else
static int ipvlan_process_v6_outbound(struct sk_buff *skb)
@@ -495,8 +495,6 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
static int ipvlan_process_outbound(struct sk_buff *skb)
{
- int ret = NET_XMIT_DROP;
-
/* The ipvlan is a pseudo-L2 device, so the packets that we receive
* will have L2; which need to discarded and processed further
* in the net-ns of the main-device.
@@ -511,7 +509,7 @@ static int ipvlan_process_outbound(struct sk_buff *skb)
"Dropped {multi|broad}cast of type=[%x]\n",
ntohs(skb->protocol));
kfree_skb(skb);
- goto out;
+ return NET_XMIT_DROP;
}
skb_pull(skb, sizeof(*ethh));
@@ -520,16 +518,14 @@ static int ipvlan_process_outbound(struct sk_buff *skb)
}
if (skb->protocol == htons(ETH_P_IPV6))
- ret = ipvlan_process_v6_outbound(skb);
+ return ipvlan_process_v6_outbound(skb);
else if (skb->protocol == htons(ETH_P_IP))
- ret = ipvlan_process_v4_outbound(skb);
- else {
- pr_warn_ratelimited("Dropped outbound packet type=%x\n",
- ntohs(skb->protocol));
- kfree_skb(skb);
- }
-out:
- return ret;
+ return ipvlan_process_v4_outbound(skb);
+
+ pr_warn_ratelimited("Dropped outbound packet type=%x\n",
+ ntohs(skb->protocol));
+ kfree_skb(skb);
+ return NET_XMIT_DROP;
}
static void ipvlan_multicast_enqueue(struct ipvl_port *port,