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.


On Wed, Sep 24, 2014 at 9:28 PM, Simon Horman
<simon.hor...@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.hor...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to