LGTM, thanks. Reviewed-by: Yifeng Sun <[email protected]>
On Mon, Oct 14, 2019 at 10:53 AM Yi-Hung Wei <[email protected]> wrote: > > This patch backports the openvswitch changes and update the compat layer > for the following upstream patch. > > commit ae0be8de9a53cda3505865c11826d8ff0640237c > Author: Michal Kubecek <[email protected]> > Date: Fri Apr 26 11:13:06 2019 +0200 > > netlink: make nla_nest_start() add NLA_F_NESTED flag > > Even if the NLA_F_NESTED flag was introduced more than 11 years ago, most > netlink based interfaces (including recently added ones) are still not > setting it in kernel generated messages. Without the flag, message parsers > not aware of attribute semantics (e.g. wireshark dissector or libmnl's > mnl_nlmsg_fprintf()) cannot recognize nested attributes and won't display > the structure of their contents. > > Unfortunately we cannot just add the flag everywhere as there may be > userspace applications which check nlattr::nla_type directly rather than > through a helper masking out the flags. Therefore the patch renames > nla_nest_start() to nla_nest_start_noflag() and introduces > nla_nest_start() > as a wrapper adding NLA_F_NESTED. The calls which add NLA_F_NESTED > manually > are rewritten to use nla_nest_start(). > > Except for changes in include/net/netlink.h, the patch was generated using > this semantic patch: > > @@ expression E1, E2; @@ > -nla_nest_start(E1, E2) > +nla_nest_start_noflag(E1, E2) > > @@ expression E1, E2; @@ > -nla_nest_start_noflag(E1, E2 | NLA_F_NESTED) > +nla_nest_start(E1, E2) > > Signed-off-by: Michal Kubecek <[email protected]> > Acked-by: Jiri Pirko <[email protected]> > Acked-by: David Ahern <[email protected]> > Signed-off-by: David S. Miller <[email protected]> > > Signed-off-by: Yi-Hung Wei <[email protected]> > --- > acinclude.m4 | 1 + > datapath/conntrack.c | 6 +++--- > datapath/datapath.c | 7 +++--- > datapath/flow_netlink.c | 33 > +++++++++++++++-------------- > datapath/linux/compat/include/net/netlink.h | 9 ++++++++ > datapath/meter.c | 8 +++---- > datapath/vport-vxlan.c | 2 +- > datapath/vport.c | 2 +- > 8 files changed, 40 insertions(+), 28 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index dca09abefa96..fe121ab9126d 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -844,6 +844,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ > OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_put_in_addr]) > OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_find_nested]) > OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_is_last]) > + OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_nest_start_noflag]) > OVS_GREP_IFELSE([$KSRC/include/linux/netlink.h], [void.*netlink_set_err], > [OVS_DEFINE([HAVE_VOID_NETLINK_SET_ERR])]) > OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netlink.h], > diff --git a/datapath/conntrack.c b/datapath/conntrack.c > index 010f9af5ffd2..b11a30965147 100644 > --- a/datapath/conntrack.c > +++ b/datapath/conntrack.c > @@ -1776,7 +1776,7 @@ static bool ovs_ct_nat_to_attr(const struct > ovs_conntrack_info *info, > { > struct nlattr *start; > > - start = nla_nest_start(skb, OVS_CT_ATTR_NAT); > + start = nla_nest_start_noflag(skb, OVS_CT_ATTR_NAT); > if (!start) > return false; > > @@ -1847,7 +1847,7 @@ int ovs_ct_action_to_attr(const struct > ovs_conntrack_info *ct_info, > { > struct nlattr *start; > > - start = nla_nest_start(skb, OVS_ACTION_ATTR_CT); > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_CT); > if (!start) > return -EMSGSIZE; > > @@ -2257,7 +2257,7 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, > struct genl_info *info) > if (IS_ERR(reply)) > return PTR_ERR(reply); > > - nla_reply = nla_nest_start(reply, OVS_CT_LIMIT_ATTR_ZONE_LIMIT); > + nla_reply = nla_nest_start_noflag(reply, > OVS_CT_LIMIT_ATTR_ZONE_LIMIT); > > if (a[OVS_CT_LIMIT_ATTR_ZONE_LIMIT]) { > err = ovs_ct_limit_get_zone_limit( > diff --git a/datapath/datapath.c b/datapath/datapath.c > index 94e4f6ffd6e9..78e2e6310529 100644 > --- a/datapath/datapath.c > +++ b/datapath/datapath.c > @@ -475,7 +475,8 @@ static int queue_userspace_packet(struct datapath *dp, > struct sk_buff *skb, > > > if (upcall_info->egress_tun_info) { > - nla = nla_nest_start(user_skb, > OVS_PACKET_ATTR_EGRESS_TUN_KEY); > + nla = nla_nest_start_noflag(user_skb, > + OVS_PACKET_ATTR_EGRESS_TUN_KEY); > if (!nla) { > err = -EMSGSIZE; > goto out; > @@ -487,7 +488,7 @@ static int queue_userspace_packet(struct datapath *dp, > struct sk_buff *skb, > } > > if (upcall_info->actions_len) { > - nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS); > + nla = nla_nest_start_noflag(user_skb, > OVS_PACKET_ATTR_ACTIONS); > if (!nla) { > err = -EMSGSIZE; > goto out; > @@ -789,7 +790,7 @@ static int ovs_flow_cmd_fill_actions(const struct sw_flow > *flow, > * This can only fail for dump operations because the skb is always > * properly sized for single flows. > */ > - start = nla_nest_start(skb, OVS_FLOW_ATTR_ACTIONS); > + start = nla_nest_start_noflag(skb, OVS_FLOW_ATTR_ACTIONS); > if (start) { > const struct sw_flow_actions *sf_acts; > > diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c > index 0f7ab53fc141..35f13d753cec 100644 > --- a/datapath/flow_netlink.c > +++ b/datapath/flow_netlink.c > @@ -839,7 +839,7 @@ static int vxlan_opt_to_nlattr(struct sk_buff *skb, > const struct vxlan_metadata *opts = tun_opts; > struct nlattr *nla; > > - nla = nla_nest_start(skb, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS); > + nla = nla_nest_start_noflag(skb, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS); > if (!nla) > return -EMSGSIZE; > > @@ -926,7 +926,7 @@ static int ip_tun_to_nlattr(struct sk_buff *skb, > struct nlattr *nla; > int err; > > - nla = nla_nest_start(skb, OVS_KEY_ATTR_TUNNEL); > + nla = nla_nest_start_noflag(skb, OVS_KEY_ATTR_TUNNEL); > if (!nla) > return -EMSGSIZE; > > @@ -1934,7 +1934,7 @@ static int nsh_key_to_nlattr(const struct ovs_key_nsh > *nsh, bool is_mask, > { > struct nlattr *start; > > - start = nla_nest_start(skb, OVS_KEY_ATTR_NSH); > + start = nla_nest_start_noflag(skb, OVS_KEY_ATTR_NSH); > if (!start) > return -EMSGSIZE; > > @@ -2017,14 +2017,15 @@ static int __ovs_nla_put_key(const struct sw_flow_key > *swkey, > if (swkey->eth.vlan.tci || eth_type_vlan(swkey->eth.type)) { > if (ovs_nla_put_vlan(skb, &output->eth.vlan, is_mask)) > goto nla_put_failure; > - encap = nla_nest_start(skb, OVS_KEY_ATTR_ENCAP); > + encap = nla_nest_start_noflag(skb, > OVS_KEY_ATTR_ENCAP); > if (!swkey->eth.vlan.tci) > goto unencap; > > if (swkey->eth.cvlan.tci || > eth_type_vlan(swkey->eth.type)) { > if (ovs_nla_put_vlan(skb, &output->eth.cvlan, > is_mask)) > goto nla_put_failure; > - in_encap = nla_nest_start(skb, > OVS_KEY_ATTR_ENCAP); > + in_encap = nla_nest_start_noflag(skb, > + > OVS_KEY_ATTR_ENCAP); > if (!swkey->eth.cvlan.tci) > goto unencap; > } > @@ -2203,7 +2204,7 @@ int ovs_nla_put_key(const struct sw_flow_key *swkey, > int err; > struct nlattr *nla; > > - nla = nla_nest_start(skb, attr); > + nla = nla_nest_start_noflag(skb, attr); > if (!nla) > return -EMSGSIZE; > err = __ovs_nla_put_key(swkey, output, is_mask, skb); > @@ -3234,7 +3235,7 @@ static int sample_action_to_attr(const struct nlattr > *attr, > const struct sample_arg *arg; > struct nlattr *actions; > > - start = nla_nest_start(skb, OVS_ACTION_ATTR_SAMPLE); > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_SAMPLE); > if (!start) > return -EMSGSIZE; > > @@ -3247,7 +3248,7 @@ static int sample_action_to_attr(const struct nlattr > *attr, > goto out; > } > > - ac_start = nla_nest_start(skb, OVS_SAMPLE_ATTR_ACTIONS); > + ac_start = nla_nest_start_noflag(skb, OVS_SAMPLE_ATTR_ACTIONS); > if (!ac_start) { > err = -EMSGSIZE; > goto out; > @@ -3273,7 +3274,7 @@ static int clone_action_to_attr(const struct nlattr > *attr, > struct nlattr *start; > int err = 0, rem = nla_len(attr); > > - start = nla_nest_start(skb, OVS_ACTION_ATTR_CLONE); > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_CLONE); > if (!start) > return -EMSGSIZE; > > @@ -3295,7 +3296,7 @@ static int check_pkt_len_action_to_attr(const struct > nlattr *attr, > const struct nlattr *a, *cpl_arg; > int err = 0, rem = nla_len(attr); > > - start = nla_nest_start(skb, OVS_ACTION_ATTR_CHECK_PKT_LEN); > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_CHECK_PKT_LEN); > if (!start) > return -EMSGSIZE; > > @@ -3314,8 +3315,8 @@ static int check_pkt_len_action_to_attr(const struct > nlattr *attr, > * 'OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL'. > */ > a = nla_next(cpl_arg, &rem); > - ac_start = nla_nest_start(skb, > - OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL); > + ac_start = nla_nest_start_noflag(skb, > + > OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL); > if (!ac_start) { > err = -EMSGSIZE; > goto out; > @@ -3333,8 +3334,8 @@ static int check_pkt_len_action_to_attr(const struct > nlattr *attr, > * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER. > */ > a = nla_next(a, &rem); > - ac_start = nla_nest_start(skb, > - OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER); > + ac_start = nla_nest_start_noflag(skb, > + > OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER); > if (!ac_start) { > err = -EMSGSIZE; > goto out; > @@ -3368,7 +3369,7 @@ static int set_action_to_attr(const struct nlattr *a, > struct sk_buff *skb) > struct ovs_tunnel_info *ovs_tun = nla_data(ovs_key); > struct ip_tunnel_info *tun_info = > &ovs_tun->tun_dst->u.tun_info; > > - start = nla_nest_start(skb, OVS_ACTION_ATTR_SET); > + start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_SET); > if (!start) > return -EMSGSIZE; > > @@ -3400,7 +3401,7 @@ static int masked_set_action_to_set_action_attr(const > struct nlattr *a, > /* Revert the conversion we did from a non-masked set action to > * masked set action. > */ > - nla = nla_nest_start(skb, OVS_ACTION_ATTR_SET); > + nla = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_SET); > if (!nla) > return -EMSGSIZE; > > diff --git a/datapath/linux/compat/include/net/netlink.h > b/datapath/linux/compat/include/net/netlink.h > index d42bf108b417..34fc3460dc81 100644 > --- a/datapath/linux/compat/include/net/netlink.h > +++ b/datapath/linux/compat/include/net/netlink.h > @@ -165,4 +165,13 @@ static inline int rpl_nla_parse(struct nlattr **tb, int > maxtype, > #define nla_parse rpl_nla_parse > #endif > > +#ifndef HAVE_NLA_NEST_START_NOFLAG > +static inline struct nlattr *rpl_nla_nest_start_noflag(struct sk_buff *skb, > + int attrtype) > +{ > + return nla_nest_start(skb, attrtype); > +} > +#define nla_nest_start_noflag rpl_nla_nest_start_noflag > +#endif > + > #endif /* net/netlink.h */ > diff --git a/datapath/meter.c b/datapath/meter.c > index eda14682fb96..b0a92891c7c0 100644 > --- a/datapath/meter.c > +++ b/datapath/meter.c > @@ -129,7 +129,7 @@ static int ovs_meter_cmd_reply_stats(struct sk_buff > *reply, u32 meter_id, > OVS_METER_ATTR_PAD)) > goto error; > > - nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS); > + nla = nla_nest_start_noflag(reply, OVS_METER_ATTR_BANDS); > if (!nla) > goto error; > > @@ -138,7 +138,7 @@ static int ovs_meter_cmd_reply_stats(struct sk_buff > *reply, u32 meter_id, > for (i = 0; i < meter->n_bands; ++i, ++band) { > struct nlattr *band_nla; > > - band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC); > + band_nla = nla_nest_start_noflag(reply, OVS_BAND_ATTR_UNSPEC); > if (!band_nla || nla_put(reply, OVS_BAND_ATTR_STATS, > sizeof(struct ovs_flow_stats), > &band->stats)) > @@ -168,11 +168,11 @@ static int ovs_meter_cmd_features(struct sk_buff *skb, > struct genl_info *info) > nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS)) > goto nla_put_failure; > > - nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS); > + nla = nla_nest_start_noflag(reply, OVS_METER_ATTR_BANDS); > if (!nla) > goto nla_put_failure; > > - band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC); > + band_nla = nla_nest_start_noflag(reply, OVS_BAND_ATTR_UNSPEC); > if (!band_nla) > goto nla_put_failure; > /* Currently only DROP band type is supported. */ > diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c > index 05764467a687..70ed376e3869 100644 > --- a/datapath/vport-vxlan.c > +++ b/datapath/vport-vxlan.c > @@ -47,7 +47,7 @@ static int vxlan_get_options(const struct vport *vport, > struct sk_buff *skb) > #endif > struct nlattr *exts; > > - exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION); > + exts = nla_nest_start_noflag(skb, OVS_TUNNEL_ATTR_EXTENSION); > if (!exts) > return -EMSGSIZE; > > diff --git a/datapath/vport.c b/datapath/vport.c > index ed7f23ec8933..f929282dcec1 100644 > --- a/datapath/vport.c > +++ b/datapath/vport.c > @@ -408,7 +408,7 @@ int ovs_vport_get_options(const struct vport *vport, > struct sk_buff *skb) > if (!vport->ops->get_options) > return 0; > > - nla = nla_nest_start(skb, OVS_VPORT_ATTR_OPTIONS); > + nla = nla_nest_start_noflag(skb, OVS_VPORT_ATTR_OPTIONS); > if (!nla) > return -EMSGSIZE; > > -- > 2.7.4 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
