From: Jakub Kicinski <jakub.kicinski@netronome•com>
To: Martin Varghese <martinvarghesenokia@gmail•com>
Cc: netdev@vger•kernel.org, pshelar@ovn•org, davem@davemloft•net,
jhs@mojatatu•com, xiyou.wangcong@gmail•com, jiri@resnulli•us,
martin.varghese@nokia•com
Subject: Re: [PATCH v2 net-next] Enhanced skb_mpls_pop to update ethertype of the packet in all the cases when an ethernet header is present is the packet.
Date: Sun, 24 Nov 2019 19:10:08 -0800 [thread overview]
Message-ID: <20191124191008.1e65f736@cakuba.netronome.com> (raw)
In-Reply-To: <1574505299-23909-1-git-send-email-martinvarghesenokia@gmail.com>
On Sat, 23 Nov 2019 16:04:59 +0530, Martin Varghese wrote:
> From: Martin Varghese <martin.varghese@nokia•com>
>
> The skb_mpls_pop was not updating ethertype of an ethernet packet if the
> packet was originally received from a non ARPHRD_ETHER device.
>
> In the below OVS data path flow, since the device corresponding to port 7
> is an l3 device (ARPHRD_NONE) the skb_mpls_pop function does not update
> the ethertype of the packet even though the previous push_eth action had
> added an ethernet header to the packet.
>
> recirc_id(0),in_port(7),eth_type(0x8847),
> mpls(label=12/0xfffff,tc=0/0,ttl=0/0x0,bos=1/1),
> actions:push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),
> pop_mpls(eth_type=0x800),4
>
> Signed-off-by: Martin Varghese <martin.varghese@nokia•com>
So this fixes all the way back to commit ed246cee09b9 ("net: core: move
pop MPLS functionality from OvS to core helper")? Please add a Fixes: tag.
> Changes in v2:
> - check for dev type removed while updating ethertype
> in function skb_mpls_pop.
> - key->mac_proto is checked in function pop_mpls to pass
> ethernt flag to skb_mpls_pop.
> - dev type is checked in function tcf_mpls_act to pass
> ethernet flag to skb_mpls_pop.
nit: changelog can be kept in the commit message for netdev patches
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 867e61d..988eefb 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5529,12 +5529,13 @@ int skb_mpls_push(struct sk_buff *skb, __be32 mpls_lse, __be16 mpls_proto,
> * @skb: buffer
> * @next_proto: ethertype of header after popped MPLS header
> * @mac_len: length of the MAC header
> - *
> + * @ethernet: flag to indicate if ethernet header is present in packet
Please don't remove the empty line between params and function
description.
> * Expects skb->data at mac header.
> *
> * Returns 0 on success, -errno otherwise.
> */
> -int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto, int mac_len)
> +int skb_mpls_pop(struct sk_buff *skb, __be16 next_proto, int mac_len,
> + bool ethernet)
> {
> int err;
>
> diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
> index 4d8c822..f919f95 100644
> --- a/net/sched/act_mpls.c
> +++ b/net/sched/act_mpls.c
> @@ -13,6 +13,7 @@
> #include <net/pkt_sched.h>
> #include <net/pkt_cls.h>
> #include <net/tc_act/tc_mpls.h>
> +#include <linux/if_arp.h>
Please retain the alphabetical order of includes.
> static unsigned int mpls_net_id;
> static struct tc_action_ops act_mpls_ops;
> @@ -76,7 +77,8 @@ static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a,
>
> switch (p->tcfm_action) {
> case TCA_MPLS_ACT_POP:
> - if (skb_mpls_pop(skb, p->tcfm_proto, mac_len))
> + if (skb_mpls_pop(skb, p->tcfm_proto, mac_len,
> + (skb->dev && skb->dev->type == ARPHRD_ETHER)))
Parenthesis unnecessary
> goto drop;
> break;
> case TCA_MPLS_ACT_PUSH:
next prev parent reply other threads:[~2019-11-25 3:10 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-23 10:34 [PATCH v2 net-next] Enhanced skb_mpls_pop to update ethertype of the packet in all the cases when an ethernet header is present is the packet Martin Varghese
2019-11-25 3:10 ` Jakub Kicinski [this message]
2019-11-25 11:02 ` Martin Varghese
2019-11-25 17:05 ` Jakub Kicinski
2019-11-25 5:32 ` Pravin Shelar
2019-11-25 10:39 ` Martin Varghese
2019-11-25 15:49 ` Martin Varghese
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=20191124191008.1e65f736@cakuba.netronome.com \
--to=jakub.kicinski@netronome$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=martin.varghese@nokia$(echo .)com \
--cc=martinvarghesenokia@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pshelar@ovn$(echo .)org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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