| Message ID | 20210823061938.28240-1-l4stpr0gr4m@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] net: bridge: replace __vlan_hwaccel_put_tag with skb_vlan_push | expand |
2021년 8월 23일 (월) 오후 6:00, Nikolay Aleksandrov <nikolay@nvidia.com>님이 작성: > > On 23/08/2021 09:19, Kangmin Park wrote: > > This changes behaviour though, I don't like changing code just for the sake of it. > Perhaps the author had a reason to use hwaccel_put_tag instead. Before we would > just put hwaccel tag, now if there already is hwaccel tag we'll push it inside > the skb and then push the new tag in hwaccel. In fact I think you can even trigger > the warning inside skb_vlan_push, so: > > Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com> > > Thanks for the review. I got it. Then, how about cleanup by changing return type of br_handle_ingress_vlan_tunnel()? This function is only referenced in br_handle_frame(), and goto drop when it return non-zero. But, the ingress function always return 0, there is no meaning for now. If you think the cleanup is worth it, I'll send you a v2 patch. Regards.
On 23/08/2021 12:12, Kangmin Park wrote: > 2021년 8월 23일 (월) 오후 6:00, Nikolay Aleksandrov <nikolay@nvidia.com>님이 작성: >> >> On 23/08/2021 09:19, Kangmin Park wrote: >> >> This changes behaviour though, I don't like changing code just for the sake of it. >> Perhaps the author had a reason to use hwaccel_put_tag instead. Before we would >> just put hwaccel tag, now if there already is hwaccel tag we'll push it inside >> the skb and then push the new tag in hwaccel. In fact I think you can even trigger >> the warning inside skb_vlan_push, so: >> >> Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com> >> >> > > Thanks for the review. I got it. > Then, how about cleanup by changing return type of > br_handle_ingress_vlan_tunnel()? > This function is only referenced in br_handle_frame(), and goto drop > when it return > non-zero. But, the ingress function always return 0, there is no > meaning for now. > If you think the cleanup is worth it, I'll send you a v2 patch. > > Regards. > Sure, I don't mind that cleanup. Cheers, Nik
diff --git a/net/bridge/br_vlan_tunnel.c b/net/bridge/br_vlan_tunnel.c index 01017448ebde..7b5a33dc9d4d 100644 --- a/net/bridge/br_vlan_tunnel.c +++ b/net/bridge/br_vlan_tunnel.c @@ -179,9 +179,7 @@ int br_handle_ingress_vlan_tunnel(struct sk_buff *skb, skb_dst_drop(skb); - __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, vlan->vid); - - return 0; + return skb_vlan_push(skb, p->br->vlan_proto, vlan->vid); } int br_handle_egress_vlan_tunnel(struct sk_buff *skb,
br_handle_ingress_vlan_tunnel() is called in br_handle_frame() and goto drop when br_handle_ingress_vlan_tunnel() return non-zero. But, br_handle_ingress_vlan_tunnel() always return 0. So, the goto routine is currently meaningless. However, paired function br_handle_egress_vlan_tunnel() call skb_vlan_pop(). So, change br_handle_ingress_vlan_tunnel() to call skb_vlan_push() instead of __vlan_hwaccel_put_tag(). And return the return value of skb_vlan_push(). Signed-off-by: Kangmin Park <l4stpr0gr4m@gmail.com> --- net/bridge/br_vlan_tunnel.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)