public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Simon Horman <simon.horman@netronome•com>
To: pravin shelar <pshelar@ovn•org>
Cc: Linux Kernel Network Developers <netdev@vger•kernel.org>,
	ovs dev <dev@openvswitch•org>
Subject: Re: [PATCH net-next v10 3/5] openvswitch: add support to push and pop mpls for layer3 packets
Date: Tue, 7 Jun 2016 11:51:33 +0900	[thread overview]
Message-ID: <20160607025130.GD31696@vergenet.net> (raw)
In-Reply-To: <CAOrHB_D0XoGUjSXztNMdektGgTMPxeVfbNF0WcTZuTbR4VYz5g@mail.gmail.com>

On Thu, Jun 02, 2016 at 03:02:00PM -0700, pravin shelar wrote:
> On Wed, Jun 1, 2016 at 11:24 PM, Simon Horman
> <simon.horman@netronome•com> wrote:
> > Allow push and pop mpls actions to act on layer 3 packets by teaching
> > them not to access non-existent L2 headers of such packets.
> >
> > Signed-off-by: Simon Horman <simon.horman@netronome•com>
> > ---
> > v10
> > * Limit scope of hdr in {push,pop}_mpls()
> >
> > v9
> > * New Patch
> > ---
> >  net/openvswitch/actions.c | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> > index 9a3eb7a0ebf4..15f130e4c22b 100644
> > --- a/net/openvswitch/actions.c
> > +++ b/net/openvswitch/actions.c
> > @@ -172,7 +172,8 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> >
> >         skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
> >
> > -       update_ethertype(skb, eth_hdr(skb), mpls->mpls_ethertype);
> > +       if (skb->mac_len)
> > +               update_ethertype(skb, eth_hdr(skb), mpls->mpls_ethertype);
> We can move all ethernet related code in this if block. for example memmove().

My assumption is that the memmove() does nothing if skb->mac_len is zero
and from my point of view it seems clean to leave it where it is unless
the code around it also moves.

Is there other code you think could/should be moved into the
if (skb->mac_len) block?

> 
> >         if (!skb->inner_protocol)
> >                 skb_set_inner_protocol(skb, skb->protocol);
> >         skb->protocol = mpls->mpls_ethertype;
> > @@ -184,7 +185,6 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> >  static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> >                     const __be16 ethertype)
> >  {
> > -       struct ethhdr *hdr;
> >         int err;
> >
> >         err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
> > @@ -199,11 +199,16 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> >         __skb_pull(skb, MPLS_HLEN);
> >         skb_reset_mac_header(skb);
> >
> > -       /* skb_mpls_header() is used to locate the ethertype
> > -        * field correctly in the presence of VLAN tags.
> > -        */
> > -       hdr = (struct ethhdr *)(skb_mpls_header(skb) - ETH_HLEN);
> > -       update_ethertype(skb, hdr, ethertype);
> > +       if (skb->mac_len) {
> > +               struct ethhdr *hdr;
> > +
> > +               /* skb_mpls_header() is used to locate the ethertype
> > +                * field correctly in the presence of VLAN tags.
> > +                */
> > +               hdr = (struct ethhdr *)(skb_mpls_header(skb) - ETH_HLEN);
> > +               update_ethertype(skb, hdr, ethertype);
> > +       }
> same here.

  reply	other threads:[~2016-06-07  2:51 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02  6:24 [PATCH net-next v10 0/5] openvswitch: support for layer 3 encapsulated packets Simon Horman
2016-06-02  6:24 ` [PATCH net-next v10 1/5] net: add skb_vlan_accel helper Simon Horman
2016-06-02 22:01   ` pravin shelar
2016-06-02  6:24 ` [PATCH net-next v10 2/5] openvswitch: set skb protocol and mac_len when receiving on internal device Simon Horman
     [not found]   ` <1464848686-7656-3-git-send-email-simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org>
2016-06-02 22:01     ` pravin shelar
2016-06-07  3:08       ` Simon Horman
     [not found]         ` <20160607030809.GE31696-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
2016-06-07 22:45           ` pravin shelar
2016-06-17  5:53             ` Simon Horman
     [not found]               ` <20160617055331.GA24833-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
2016-06-19  1:38                 ` pravin shelar
     [not found]                   ` <CAOrHB_BWfLk8yba2XkF43Hm-czGUZnVWQQ=HW3-vvpFEH7GNpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-21  2:25                     ` Simon Horman
     [not found]                       ` <20160621022458.GA28358-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
2016-06-21 16:30                         ` pravin shelar
2016-06-23  2:04                           ` Simon Horman
2016-06-27  9:35                             ` Jiri Benc
2016-06-02  6:24 ` [PATCH net-next v10 3/5] openvswitch: add support to push and pop mpls for layer3 packets Simon Horman
2016-06-02 22:02   ` pravin shelar
2016-06-07  2:51     ` Simon Horman [this message]
2016-06-07 22:45       ` pravin shelar
2016-06-02  6:24 ` [PATCH net-next v10 4/5] openvswitch: add layer 3 flow/port support Simon Horman
     [not found]   ` <1464848686-7656-5-git-send-email-simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org>
2016-06-02 22:02     ` pravin shelar
2016-06-07  2:46       ` Simon Horman
     [not found]         ` <20160607024609.GC31696-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
2016-06-07 22:45           ` pravin shelar
2016-06-17  6:53             ` Simon Horman
2016-06-02  6:24 ` [PATCH net-next v10 5/5] openvswitch: use ipgre tunnel rather than gretap tunnel Simon Horman

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=20160607025130.GD31696@vergenet.net \
    --to=simon.horman@netronome$(echo .)com \
    --cc=dev@openvswitch$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=pshelar@ovn$(echo .)org \
    /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