On Fri, Oct 17, 2014 at 12:16 AM, Simon Horman
<simon.hor...@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.
>>
>>
>> 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
