[ovs-dev] [PATCH] datapath: Rename last_action() as nla_is_last() and move to netlink.h

Andy Zhou azhou at nicira.com
Fri Oct 17 20:29:38 UTC 2014


Thanks.

On Fri, Oct 17, 2014 at 1:24 PM, Simon Horman
<simon.horman at netronome.com> wrote:
>
> 2014/10/17 19:25 "Pravin Shelar" <pshelar at nicira.com>:
>>
>> On Fri, Oct 17, 2014 at 12:16 AM, Simon Horman
>> <simon.horman at netronome.com> wrote:
>> > On Thu, Oct 16, 2014 at 04:50:10PM -0700, Andy Zhou wrote:
>> >> Simon, The change makes a lot of sense.
>> >>
>> >> I am just wondering if we should upstream the netlink.h change first?
>> >> To me, it seems to add a new compat API that does not exist upstream.
>> >
>> > Sure, I think that makes sense. Though I'm not sure if upstream likes
>> > to take new API calls that aren't used. Perhaps a good way forwards
>> > would be for me to re-submit this patch against the upstream net-next
>> > kernel.
>> >
>> > Pravin, how would you feel about that?
>> >
>> Yes, lets submit it upstream and then backport it.
>
> Thanks, I'll cook up a patch for upstream.
>
>> >> On Wed, Sep 24, 2014 at 9:28 PM, Simon Horman
>> >> <simon.horman at netronome.com> wrote:
>> >> > The original motivation for this change was to allow the
>> >> > helper to be used in files other than actions.c as part
>> >> > of work on an odp select group action.
>> >> >
>> >> > It was as pointed out by Thomas Graf that this helper would be best
>> >> > off living in netlink.h. Furthermore, I think that the generic nature
>> >> > of this
>> >> > helper means it is best off in netlink.h regardless of if it is used
>> >> > more than
>> >> > one .c file or not. Thus I would like it considered independent
>> >> > of the work on an odp select group action.
>> >> >
>> >> > Signed-off-by: Simon Horman <simon.horman at netronome.com>
>> >> > ---
>> >> >  datapath/actions.c                          | 11 +++--------
>> >> >  datapath/linux/compat/include/net/netlink.h |  4 ++++
>> >> >  2 files changed, 7 insertions(+), 8 deletions(-)
>> >> >
>> >> > diff --git a/datapath/actions.c b/datapath/actions.c
>> >> > index b527cb6..535b87e 100644
>> >> > --- a/datapath/actions.c
>> >> > +++ b/datapath/actions.c
>> >> > @@ -671,11 +671,6 @@ static int output_userspace(struct datapath *dp,
>> >> > struct sk_buff *skb,
>> >> >         return ovs_dp_upcall(dp, skb, key, &upcall);
>> >> >  }
>> >> >
>> >> > -static bool last_action(const struct nlattr *a, int rem)
>> >> > -{
>> >> > -       return a->nla_len == rem;
>> >> > -}
>> >> > -
>> >> >  static int sample(struct datapath *dp, struct sk_buff *skb,
>> >> >                   struct sw_flow_key *key, const struct nlattr *attr)
>> >> >  {
>> >> > @@ -710,7 +705,7 @@ static int sample(struct datapath *dp, struct
>> >> > sk_buff *skb,
>> >> >          * user space. This skb will be consumed by its caller.
>> >> >          */
>> >> >         if (likely(nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
>> >> > -                  last_action(a, rem)))
>> >> > +                  nla_is_last(a, rem)))
>> >> >                 return output_userspace(dp, skb, key, a);
>> >> >
>> >> >         skb = skb_clone(skb, GFP_ATOMIC);
>> >> > @@ -810,7 +805,7 @@ static int execute_recirc(struct datapath *dp,
>> >> > struct sk_buff *skb,
>> >> >         }
>> >> >         BUG_ON(!is_flow_key_valid(key));
>> >> >
>> >> > -       if (!last_action(a, rem)) {
>> >> > +       if (!nla_is_last(a, rem)) {
>> >> >                 /* Recirc action is the not the last action
>> >> >                  * of the action list, need to clone the skb.
>> >> >                  */
>> >> > @@ -897,7 +892,7 @@ static int do_execute_actions(struct datapath
>> >> > *dp, struct sk_buff *skb,
>> >> >
>> >> >                 case OVS_ACTION_ATTR_RECIRC:
>> >> >                         err = execute_recirc(dp, skb, key, a, rem);
>> >> > -                       if (last_action(a, rem)) {
>> >> > +                       if (nla_is_last(a, rem)) {
>> >> >                                 /* If this is the last action, the
>> >> > skb has
>> >> >                                  * been consumed or freed.
>> >> >                                  * Return immediately.
>> >> > diff --git a/datapath/linux/compat/include/net/netlink.h
>> >> > b/datapath/linux/compat/include/net/netlink.h
>> >> > index a6dc584..de37058 100644
>> >> > --- a/datapath/linux/compat/include/net/netlink.h
>> >> > +++ b/datapath/linux/compat/include/net/netlink.h
>> >> > @@ -63,4 +63,8 @@ static inline struct nlattr *nla_find_nested(struct
>> >> > nlattr *nla, int attrtype)
>> >> >  }
>> >> >  #endif
>> >> >
>> >> > +static inline bool nla_is_last(const struct nlattr *a, int rem)
>> >> > +{
>> >> > +       return a->nla_len == rem;
>> >> > +}
>> >> >  #endif /* net/netlink.h */
>> >> > --
>> >> > 2.0.1
>> >> >
>> >> > _______________________________________________
>> >> > dev mailing list
>> >> > dev at openvswitch.org
>> >> > http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list