* Re: [PATCHv11 net-next 2/2] openvswitch: Add support for unique flow IDs.
From: Pravin Shelar @ 2014-12-09 18:32 UTC (permalink / raw)
To: Joe Stringer; +Cc: netdev, LKML, dev@openvswitch.org
In-Reply-To: <1417575363-13770-2-git-send-email-joestringer@nicira.com>
On Tue, Dec 2, 2014 at 6:56 PM, Joe Stringer <joestringer@nicira.com> wrote:
> Previously, flows were manipulated by userspace specifying a full,
> unmasked flow key. This adds significant burden onto flow
> serialization/deserialization, particularly when dumping flows.
>
> This patch adds an alternative way to refer to flows using a
> variable-length "unique flow identifier" (UFID). At flow setup time,
> userspace may specify a UFID for a flow, which is stored with the flow
> and inserted into a separate table for lookup, in addition to the
> standard flow table. Flows created using a UFID must be fetched or
> deleted using the UFID.
>
> All flow dump operations may now be made more terse with OVS_UFID_F_*
> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
> omit the flow key from a datapath operation if the flow has a
> corresponding UFID. This significantly reduces the time spent assembling
> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
> enabled, the datapath only returns the UFID and statistics for each flow
> during flow dump, increasing ovs-vswitchd revalidator performance by up
> to 50%.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
Thanks for updating patch against net-next.
> ---
> v11: Separate UFID and unmasked key from sw_flow.
> Modify interface to remove nested UFID attributes.
> Only allow UFIDs between 1-256 octets.
> Move UFID nla fetch helpers to flow_netlink.h.
> Perform complete nlmsg_parsing in ovs_flow_cmd_dump().
> Check UFID table for flows with duplicate UFID at flow setup.
> Tidy up mask/key/ufid insertion into flow_table.
> Rebase.
> v10: Ignore flow_key in requests if UFID is specified.
> Only allow UFID flows to be indexed by UFID.
> Only allow non-UFID flows to be indexed by unmasked flow key.
> Unite the unmasked_key and ufid+ufid_hash in 'struct sw_flow'.
> Don't periodically rehash the UFID table.
> Resize the UFID table independently from the flow table.
> Modify table_destroy() to iterate once and delete from both tables.
> Fix UFID memory leak in flow_free().
> Remove kernel-only UFIDs for non-UFID cases.
> Rename "OVS_UFID_F_SKIP_*" -> "OVS_UFID_F_OMIT_*"
> Update documentation.
> Rebase.
> v9: No change.
> v8: Rename UID -> UFID "unique flow identifier".
> Fix null dereference when adding flow without uid or mask.
> If UFID and not match are specified, and lookup fails, return ENOENT.
> Rebase.
> v7: Remove OVS_DP_F_INDEX_BY_UID.
> Rework UID serialisation for variable-length UID.
> Log error if uid not specified and OVS_UID_F_SKIP_KEY is set.
> Rebase against "probe" logging changes.
> v6: Fix documentation for supporting UIDs between 32-128 bits.
> Minor style fixes.
> Rebase.
> v5: No change.
> v4: Fix memory leaks.
> Log when triggering the older userspace issue above.
> v3: Initial post.
> ---
> Documentation/networking/openvswitch.txt | 13 ++
> include/uapi/linux/openvswitch.h | 20 +++
> net/openvswitch/datapath.c | 241 +++++++++++++++++++-----------
> net/openvswitch/flow.h | 16 +-
> net/openvswitch/flow_netlink.c | 63 +++++++-
> net/openvswitch/flow_netlink.h | 4 +
> net/openvswitch/flow_table.c | 204 +++++++++++++++++++------
> net/openvswitch/flow_table.h | 7 +
> 8 files changed, 437 insertions(+), 131 deletions(-)
>
> diff --git a/Documentation/networking/openvswitch.txt b/Documentation/networking/openvswitch.txt
> index 37c20ee..b3b9ac6 100644
> --- a/Documentation/networking/openvswitch.txt
> +++ b/Documentation/networking/openvswitch.txt
> @@ -131,6 +131,19 @@ performs best-effort detection of overlapping wildcarded flows and may reject
> some but not all of them. However, this behavior may change in future versions.
>
>
> +Unique flow identifiers
> +-----------------------
> +
> +An alternative to using the original match portion of a key as the handle for
> +flow identification is a unique flow identifier, or "UFID". UFIDs are optional
> +for both the kernel and user space program.
> +
> +User space programs that support UFID are expected to provide it during flow
> +setup in addition to the flow, then refer to the flow using the UFID for all
> +future operations. The kernel is not required to index flows by the original
> +flow key if a UFID is specified.
> +
> +
> Basic rule for evolving flow keys
> ---------------------------------
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 3a6dcaa..80db129 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -444,6 +444,14 @@ struct ovs_key_nd {
> * a wildcarded match. Omitting attribute is treated as wildcarding all
> * corresponding fields. Optional for all requests. If not present,
> * all flow key bits are exact match bits.
> + * @OVS_FLOW_ATTR_UFID: A value between 1-256 octets specifying a unique
> + * identifier for the flow. Causes the flow to be indexed by this value rather
> + * than the value of the %OVS_FLOW_ATTR_KEY attribute. Optional for all
> + * requests. Present in notifications if the flow was created with this
> + * attribute.
> + * @OVS_FLOW_ATTR_UFID_FLAGS: A 32-bit value of OR'd %OVS_UFID_F_*
> + * flags that provide alternative semantics for flow installation and
> + * retrieval. Optional for all requests.
> *
> * These attributes follow the &struct ovs_header within the Generic Netlink
> * payload for %OVS_FLOW_* commands.
> @@ -459,12 +467,24 @@ enum ovs_flow_attr {
> OVS_FLOW_ATTR_MASK, /* Sequence of OVS_KEY_ATTR_* attributes. */
> OVS_FLOW_ATTR_PROBE, /* Flow operation is a feature probe, error
> * logging should be suppressed. */
> + OVS_FLOW_ATTR_UFID, /* Variable length unique flow identifier. */
> + OVS_FLOW_ATTR_UFID_FLAGS,/* u32 of OVS_UFID_F_*. */
> __OVS_FLOW_ATTR_MAX
> };
>
> #define OVS_FLOW_ATTR_MAX (__OVS_FLOW_ATTR_MAX - 1)
>
> /**
> + * Omit attributes for notifications.
> + *
> + * If a datapath request contains an %OVS_UFID_F_OMIT_* flag, then the datapath
> + * may omit the corresponding %OVS_FLOW_ATTR_* from the response.
> + */
> +#define OVS_UFID_F_OMIT_KEY (1 << 0)
> +#define OVS_UFID_F_OMIT_MASK (1 << 1)
> +#define OVS_UFID_F_OMIT_ACTIONS (1 << 2)
> +
> +/**
> * enum ovs_sample_attr - Attributes for %OVS_ACTION_ATTR_SAMPLE action.
> * @OVS_SAMPLE_ATTR_PROBABILITY: 32-bit fraction of packets to sample with
> * @OVS_ACTION_ATTR_SAMPLE. A value of 0 samples no packets, a value of
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index b2a3796..d54e920 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -65,6 +65,8 @@ static struct genl_family dp_packet_genl_family;
> static struct genl_family dp_flow_genl_family;
> static struct genl_family dp_datapath_genl_family;
>
> +static const struct nla_policy flow_policy[];
> +
> static const struct genl_multicast_group ovs_dp_flow_multicast_group = {
> .name = OVS_FLOW_MCGROUP,
> };
> @@ -662,11 +664,18 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
> }
> }
>
> -static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
> +static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts,
> + const struct sw_flow_id *sfid)
> {
> + size_t sfid_len = 0;
> +
> + if (sfid && sfid->ufid_len)
> + sfid_len = nla_total_size(sfid->ufid_len);
> +
Can you also use ufid_flags to determine exact msg size?
> return NLMSG_ALIGN(sizeof(struct ovs_header))
> + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_KEY */
> + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_MASK */
> + + sfid_len /* OVS_FLOW_ATTR_UFID */
> + nla_total_size(sizeof(struct ovs_flow_stats)) /* OVS_FLOW_ATTR_STATS */
> + nla_total_size(1) /* OVS_FLOW_ATTR_TCP_FLAGS */
> + nla_total_size(8) /* OVS_FLOW_ATTR_USED */
> @@ -741,7 +750,7 @@ static int ovs_flow_cmd_fill_actions(const struct sw_flow *flow,
> /* Called with ovs_mutex or RCU read lock. */
> static int ovs_flow_cmd_fill_info(const struct sw_flow *flow, int dp_ifindex,
> struct sk_buff *skb, u32 portid,
> - u32 seq, u32 flags, u8 cmd)
> + u32 seq, u32 flags, u8 cmd, u32 ufid_flags)
> {
> const int skb_orig_len = skb->len;
> struct ovs_header *ovs_header;
> @@ -754,21 +763,35 @@ static int ovs_flow_cmd_fill_info(const struct sw_flow *flow, int dp_ifindex,
>
> ovs_header->dp_ifindex = dp_ifindex;
>
> - err = ovs_nla_put_unmasked_key(flow, skb);
> + if (flow->ufid)
> + err = nla_put(skb, OVS_FLOW_ATTR_UFID, flow->ufid->ufid_len,
> + flow->ufid->ufid);
> + else
> + err = ovs_nla_put_unmasked_key(flow, skb);
> if (err)
> goto error;
>
> - err = ovs_nla_put_mask(flow, skb);
> - if (err)
> - goto error;
> + if (!(ufid_flags & OVS_UFID_F_OMIT_KEY) && flow->ufid) {
> + err = ovs_nla_put_masked_key(flow, skb);
> + if (err)
> + goto error;
> + }
> +
> + if (!(ufid_flags & OVS_UFID_F_OMIT_MASK)) {
> + err = ovs_nla_put_mask(flow, skb);
> + if (err)
> + goto error;
> + }
>
> err = ovs_flow_cmd_fill_stats(flow, skb);
> if (err)
> goto error;
>
> - err = ovs_flow_cmd_fill_actions(flow, skb, skb_orig_len);
> - if (err)
> - goto error;
> + if (!(ufid_flags & OVS_UFID_F_OMIT_ACTIONS)) {
> + err = ovs_flow_cmd_fill_actions(flow, skb, skb_orig_len);
> + if (err)
> + goto error;
> + }
>
> return genlmsg_end(skb, ovs_header);
>
> @@ -779,6 +802,7 @@ error:
>
> /* May not be called with RCU read lock. */
> static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *acts,
> + const struct sw_flow_id *sfid,
> struct genl_info *info,
> bool always)
> {
> @@ -787,7 +811,8 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *act
> if (!always && !ovs_must_notify(&dp_flow_genl_family, info, 0))
> return NULL;
>
> - skb = genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info, GFP_KERNEL);
> + skb = genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts, sfid), info,
> + GFP_KERNEL);
> if (!skb)
> return ERR_PTR(-ENOMEM);
>
> @@ -798,19 +823,19 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *act
> static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
> int dp_ifindex,
> struct genl_info *info, u8 cmd,
> - bool always)
> + bool always, u32 ufid_flags)
> {
> struct sk_buff *skb;
> int retval;
>
> - skb = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info,
> - always);
> + skb = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts),
> + flow->ufid, info, always);
> if (IS_ERR_OR_NULL(skb))
> return skb;
>
> retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
> info->snd_portid, info->snd_seq, 0,
> - cmd);
> + cmd, ufid_flags);
> BUG_ON(retval < 0);
> return skb;
> }
> @@ -819,12 +844,14 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> {
> struct nlattr **a = info->attrs;
> struct ovs_header *ovs_header = info->userhdr;
> - struct sw_flow *flow, *new_flow;
> + struct sw_flow *flow = NULL, *new_flow;
> struct sw_flow_mask mask;
> struct sk_buff *reply;
> struct datapath *dp;
> + struct sw_flow_key key;
> struct sw_flow_actions *acts;
> struct sw_flow_match match;
> + u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> int error;
> bool log = !a[OVS_FLOW_ATTR_PROBE];
>
> @@ -849,13 +876,30 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> }
>
> /* Extract key. */
> - ovs_match_init(&match, &new_flow->unmasked_key, &mask);
> + ovs_match_init(&match, &key, &mask);
> error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
> a[OVS_FLOW_ATTR_MASK], log);
> if (error)
> goto err_kfree_flow;
>
> - ovs_flow_mask_key(&new_flow->key, &new_flow->unmasked_key, &mask);
> + ovs_flow_mask_key(&new_flow->key, &key, &mask);
> +
> + /* Extract flow id. */
> + error = ovs_nla_copy_ufid(a[OVS_FLOW_ATTR_UFID], &new_flow->ufid, log);
> + if (error)
> + goto err_kfree_flow;
Returning zero in case of no UFID is strange. Can you check for UFID
attribute and then copy ufid unconditionally?
> + if (!new_flow->ufid) {
> + struct sw_flow_key *new_key;
> +
> + new_key = kmalloc(sizeof(*new_flow->unmasked_key), GFP_KERNEL);
> + if (new_key) {
> + memcpy(new_key, &key, sizeof(key));
> + new_flow->unmasked_key = new_key;
> + } else {
> + error = -ENOMEM;
> + goto err_kfree_flow;
> + }
> + }
>
> /* Validate actions. */
> error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &new_flow->key,
> @@ -865,7 +909,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> goto err_kfree_flow;
> }
>
> - reply = ovs_flow_cmd_alloc_info(acts, info, false);
> + reply = ovs_flow_cmd_alloc_info(acts, new_flow->ufid, info, false);
> if (IS_ERR(reply)) {
> error = PTR_ERR(reply);
> goto err_kfree_acts;
> @@ -877,8 +921,12 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> error = -ENODEV;
> goto err_unlock_ovs;
> }
> +
> /* Check if this is a duplicate flow */
> - flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->unmasked_key);
> + if (new_flow->ufid)
> + flow = ovs_flow_tbl_lookup_ufid(&dp->table, new_flow->ufid);
> + if (!flow)
> + flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->key);
Need tight checking, for example what if flow with UFID does not exist
in UFID table but it exist in flow-table? This can complicate
flow-update case where we can not assume UFID of new and old flow is
same.
> if (likely(!flow)) {
> rcu_assign_pointer(new_flow->sf_acts, acts);
>
> @@ -894,7 +942,8 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> ovs_header->dp_ifindex,
> reply, info->snd_portid,
> info->snd_seq, 0,
> - OVS_FLOW_CMD_NEW);
> + OVS_FLOW_CMD_NEW,
> + ufid_flags);
> BUG_ON(error < 0);
> }
> ovs_unlock();
> @@ -912,11 +961,13 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> error = -EEXIST;
> goto err_unlock_ovs;
> }
> - /* The unmasked key has to be the same for flow updates. */
> - if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) {
> - /* Look for any overlapping flow. */
> + /* The flow identifier has to be the same for flow updates.
> + * Look for any overlapping flow.
> + */
> + if (!flow->ufid &&
> + unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) {
> flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> - if (!flow) {
> + if (unlikely(!flow)) {
> error = -ENOENT;
> goto err_unlock_ovs;
> }
> @@ -930,7 +981,8 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> ovs_header->dp_ifindex,
> reply, info->snd_portid,
> info->snd_seq, 0,
> - OVS_FLOW_CMD_NEW);
> + OVS_FLOW_CMD_NEW,
> + ufid_flags);
> BUG_ON(error < 0);
> }
> ovs_unlock();
> @@ -980,45 +1032,34 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
> struct nlattr **a = info->attrs;
> struct ovs_header *ovs_header = info->userhdr;
> struct sw_flow_key key;
> - struct sw_flow *flow;
> + struct sw_flow *flow = NULL;
> struct sw_flow_mask mask;
> struct sk_buff *reply = NULL;
> struct datapath *dp;
> struct sw_flow_actions *old_acts = NULL, *acts = NULL;
> struct sw_flow_match match;
> + struct sw_flow_id *ufid;
> + u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> int error;
> bool log = !a[OVS_FLOW_ATTR_PROBE];
>
> - /* Extract key. */
> - error = -EINVAL;
> - if (!a[OVS_FLOW_ATTR_KEY]) {
> + /* Extract identifier. Take a copy to avoid "Wframe-larger-than=1024"
> + * warning.
> + */
What if we limit the implementation of UFID to max 128 bit, does it
still gives you the warning?
> + error = ovs_nla_copy_ufid(a[OVS_FLOW_ATTR_UFID], &ufid, log);
> + if (error)
> + return error;
> + if (a[OVS_FLOW_ATTR_KEY]) {
> + ovs_match_init(&match, &key, &mask);
> + error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
> + a[OVS_FLOW_ATTR_MASK], log);
> + } else if (!ufid) {
> OVS_NLERR(log, "Flow key attribute not present in set flow.");
> - goto error;
> + error = -EINVAL;
> }
> -
> - ovs_match_init(&match, &key, &mask);
> - error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
> - a[OVS_FLOW_ATTR_MASK], log);
> if (error)
> goto error;
>
> - /* Validate actions. */
> - if (a[OVS_FLOW_ATTR_ACTIONS]) {
> - acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &key, &mask,
> - log);
> - if (IS_ERR(acts)) {
> - error = PTR_ERR(acts);
> - goto error;
> - }
> -
> - /* Can allocate before locking if have acts. */
> - reply = ovs_flow_cmd_alloc_info(acts, info, false);
> - if (IS_ERR(reply)) {
> - error = PTR_ERR(reply);
> - goto err_kfree_acts;
> - }
> - }
> -
Why are you moving this action validation under ovs-lock?
> ovs_lock();
> dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
> if (unlikely(!dp)) {
> @@ -1026,33 +1067,34 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
> goto err_unlock_ovs;
> }
> /* Check that the flow exists. */
> - flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> + if (ufid)
> + flow = ovs_flow_tbl_lookup_ufid(&dp->table, ufid);
> + else
> + flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> if (unlikely(!flow)) {
> error = -ENOENT;
> goto err_unlock_ovs;
> }
>
> - /* Update actions, if present. */
> - if (likely(acts)) {
> + /* Validate and update actions. */
> + if (a[OVS_FLOW_ATTR_ACTIONS]) {
> + acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &flow->key,
> + flow->mask, log);
> + if (IS_ERR(acts)) {
> + error = PTR_ERR(acts);
> + goto err_unlock_ovs;
> + }
> +
> old_acts = ovsl_dereference(flow->sf_acts);
> rcu_assign_pointer(flow->sf_acts, acts);
> + }
>
> - if (unlikely(reply)) {
> - error = ovs_flow_cmd_fill_info(flow,
> - ovs_header->dp_ifindex,
> - reply, info->snd_portid,
> - info->snd_seq, 0,
> - OVS_FLOW_CMD_NEW);
> - BUG_ON(error < 0);
> - }
> - } else {
> - /* Could not alloc without acts before locking. */
> - reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
> - info, OVS_FLOW_CMD_NEW, false);
> - if (unlikely(IS_ERR(reply))) {
> - error = PTR_ERR(reply);
> - goto err_unlock_ovs;
> - }
> + reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
> + info, OVS_FLOW_CMD_NEW, false,
> + ufid_flags);
> + if (unlikely(IS_ERR(reply))) {
> + error = PTR_ERR(reply);
> + goto err_unlock_ovs;
> }
>
> /* Clear stats. */
> @@ -1070,9 +1112,9 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
> err_unlock_ovs:
> ovs_unlock();
> kfree_skb(reply);
> -err_kfree_acts:
> kfree(acts);
> error:
> + kfree(ufid);
> return error;
> }
>
> @@ -1085,17 +1127,23 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
> struct sw_flow *flow;
> struct datapath *dp;
> struct sw_flow_match match;
> + struct sw_flow_id ufid;
> + u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> int err;
> bool log = !a[OVS_FLOW_ATTR_PROBE];
>
> - if (!a[OVS_FLOW_ATTR_KEY]) {
> + err = ovs_nla_get_ufid(a[OVS_FLOW_ATTR_UFID], &ufid, log);
> + if (err)
> + return err;
> + if (a[OVS_FLOW_ATTR_KEY]) {
> + ovs_match_init(&match, &key, NULL);
> + err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL,
> + log);
> + } else if (!ufid.ufid_len) {
> OVS_NLERR(log,
> "Flow get message rejected, Key attribute missing.");
> - return -EINVAL;
> + err = -EINVAL;
> }
> -
> - ovs_match_init(&match, &key, NULL);
> - err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL, log);
> if (err)
> return err;
>
> @@ -1106,14 +1154,17 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
> goto unlock;
> }
>
> - flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> + if (ufid.ufid_len)
> + flow = ovs_flow_tbl_lookup_ufid(&dp->table, &ufid);
> + else
> + flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> if (!flow) {
> err = -ENOENT;
> goto unlock;
> }
>
> reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex, info,
> - OVS_FLOW_CMD_NEW, true);
> + OVS_FLOW_CMD_NEW, true, ufid_flags);
> if (IS_ERR(reply)) {
> err = PTR_ERR(reply);
> goto unlock;
> @@ -1132,13 +1183,18 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
> struct ovs_header *ovs_header = info->userhdr;
> struct sw_flow_key key;
> struct sk_buff *reply;
> - struct sw_flow *flow;
> + struct sw_flow *flow = NULL;
> struct datapath *dp;
> struct sw_flow_match match;
> + struct sw_flow_id ufid;
> + u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> int err;
> bool log = !a[OVS_FLOW_ATTR_PROBE];
>
> - if (likely(a[OVS_FLOW_ATTR_KEY])) {
> + err = ovs_nla_get_ufid(a[OVS_FLOW_ATTR_UFID], &ufid, log);
> + if (err)
> + return err;
> + if (a[OVS_FLOW_ATTR_KEY]) {
> ovs_match_init(&match, &key, NULL);
> err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL,
> log);
> @@ -1153,12 +1209,15 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
> goto unlock;
> }
>
> - if (unlikely(!a[OVS_FLOW_ATTR_KEY])) {
> + if (unlikely(!a[OVS_FLOW_ATTR_KEY] && !ufid.ufid_len)) {
> err = ovs_flow_tbl_flush(&dp->table);
> goto unlock;
> }
>
> - flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> + if (ufid.ufid_len)
> + flow = ovs_flow_tbl_lookup_ufid(&dp->table, &ufid);
> + else
> + flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
> if (unlikely(!flow)) {
> err = -ENOENT;
> goto unlock;
> @@ -1168,14 +1227,15 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
> ovs_unlock();
>
> reply = ovs_flow_cmd_alloc_info((const struct sw_flow_actions __force *) flow->sf_acts,
> - info, false);
> + flow->ufid, info, false);
> if (likely(reply)) {
> if (likely(!IS_ERR(reply))) {
> rcu_read_lock(); /*To keep RCU checker happy. */
> err = ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex,
> reply, info->snd_portid,
> info->snd_seq, 0,
> - OVS_FLOW_CMD_DEL);
> + OVS_FLOW_CMD_DEL,
> + ufid_flags);
> rcu_read_unlock();
> BUG_ON(err < 0);
>
> @@ -1194,9 +1254,18 @@ unlock:
>
> static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
> {
> + struct nlattr *a[__OVS_FLOW_ATTR_MAX];
> struct ovs_header *ovs_header = genlmsg_data(nlmsg_data(cb->nlh));
> struct table_instance *ti;
> struct datapath *dp;
> + u32 ufid_flags;
> + int err;
> +
> + err = nlmsg_parse(cb->nlh, GENL_HDRLEN + dp_flow_genl_family.hdrsize,
> + a, dp_flow_genl_family.maxattr, flow_policy);
Can you add genl helper function for this?
> + if (err)
> + return err;
> + ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
>
> rcu_read_lock();
> dp = get_dp_rcu(sock_net(skb->sk), ovs_header->dp_ifindex);
> @@ -1219,7 +1288,7 @@ static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
> if (ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex, skb,
> NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq, NLM_F_MULTI,
> - OVS_FLOW_CMD_NEW) < 0)
> + OVS_FLOW_CMD_NEW, ufid_flags) < 0)
> break;
>
> cb->args[0] = bucket;
> @@ -1235,6 +1304,8 @@ static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = {
> [OVS_FLOW_ATTR_ACTIONS] = { .type = NLA_NESTED },
> [OVS_FLOW_ATTR_CLEAR] = { .type = NLA_FLAG },
> [OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG },
> + [OVS_FLOW_ATTR_UFID] = { .type = NLA_UNSPEC },
> + [OVS_FLOW_ATTR_UFID_FLAGS] = { .type = NLA_U32 },
> };
>
> static const struct genl_ops dp_flow_genl_ops[] = {
> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
> index a8b30f3..7f31dbf 100644
> --- a/net/openvswitch/flow.h
> +++ b/net/openvswitch/flow.h
> @@ -197,6 +197,13 @@ struct sw_flow_match {
> struct sw_flow_mask *mask;
> };
>
> +#define MAX_UFID_LENGTH 256
> +
For now we can limit this to 128 bits.
> +struct sw_flow_id {
> + u32 ufid_len;
> + u32 ufid[MAX_UFID_LENGTH / 4];
> +};
> +
> struct sw_flow_actions {
> struct rcu_head rcu;
> u32 actions_len;
> @@ -213,13 +220,16 @@ struct flow_stats {
>
> struct sw_flow {
> struct rcu_head rcu;
> - struct hlist_node hash_node[2];
> - u32 hash;
> + struct {
> + struct hlist_node node[2];
> + u32 hash;
> + } flow_hash, ufid_hash;
I am not sure about _hash suffix here, Can you explain it? I think
_table is better.
> int stats_last_writer; /* NUMA-node id of the last writer on
> * 'stats[0]'.
> */
> struct sw_flow_key key;
> - struct sw_flow_key unmasked_key;
> + struct sw_flow_id *ufid;
Lets move this structure inside sw_flow, so that we can avoid one
kmalloc during flow-install in case of UFID. something like:
struct {
u32 ufid_len;
union {
u32 ufid[MAX_UFID_LENGTH / 4];
struct sw_flow_key *unmasked_key;
}
} id;
> + struct sw_flow_key *unmasked_key; /* Only valid if 'ufid' is NULL. */
> struct sw_flow_mask *mask;
> struct sw_flow_actions __rcu *sf_acts;
> struct flow_stats __rcu *stats[]; /* One for each NUMA node. First one
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 7bb571f..56a5d2e 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -1095,6 +1095,67 @@ free_newmask:
> return err;
> }
>
> +static size_t get_ufid_size(const struct nlattr *attr, bool log)
> +{
> + if (!attr)
> + return 0;
> + if (!nla_len(attr)) {
> + OVS_NLERR(log, "Flow ufid must be at least 1 octet");
> + return -EINVAL;
> + }
> + if (nla_len(attr) >= MAX_UFID_LENGTH) {
> + OVS_NLERR(log, "Flow ufid size %u bytes exceeds max",
> + nla_len(attr));
> + return -EINVAL;
> + }
> +
> + return nla_len(attr);
> +}
> +
> +/* Initializes 'flow->ufid'. */
> +int ovs_nla_get_ufid(const struct nlattr *attr, struct sw_flow_id *sfid,
> + bool log)
> +{
> + size_t len;
> +
> + sfid->ufid_len = 0;
> + len = get_ufid_size(attr, log);
> + if (len <= 0)
> + return len;
> +
> + sfid->ufid_len = len;
> + memcpy(sfid->ufid, nla_data(attr), len);
> +
> + return 0;
> +}
> +
> +int ovs_nla_copy_ufid(const struct nlattr *attr, struct sw_flow_id **sfid,
> + bool log)
> +{
> + struct sw_flow_id *new_sfid = NULL;
> + size_t len;
> +
> + *sfid = NULL;
> + len = get_ufid_size(attr, log);
> + if (len <= 0)
> + return len;
> +
> + new_sfid = kmalloc(sizeof(*new_sfid), GFP_KERNEL);
> + if (!new_sfid)
> + return -ENOMEM;
> +
> + new_sfid->ufid_len = len;
> + memcpy(new_sfid->ufid, nla_data(attr), len);
> + *sfid = new_sfid;
> +
> + return 0;
> +}
> +
> +u32 ovs_nla_get_ufid_flags(const struct nlattr *attr)
> +{
> + return attr ? nla_get_u32(attr) : 0;
> +}
> +
> /**
> * ovs_nla_get_flow_metadata - parses Netlink attributes into a flow key.
> * @key: Receives extracted in_port, priority, tun_key and skb_mark.
> @@ -1367,7 +1428,7 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
> /* Called with ovs_mutex or RCU read lock. */
> int ovs_nla_put_unmasked_key(const struct sw_flow *flow, struct sk_buff *skb)
> {
> - return ovs_nla_put_flow(&flow->unmasked_key, &flow->unmasked_key,
> + return ovs_nla_put_flow(flow->unmasked_key, flow->unmasked_key,
> OVS_FLOW_ATTR_KEY, false, skb);
> }
>
> diff --git a/net/openvswitch/flow_netlink.h b/net/openvswitch/flow_netlink.h
> index ea54564..4f1bd7a 100644
> --- a/net/openvswitch/flow_netlink.h
> +++ b/net/openvswitch/flow_netlink.h
> @@ -57,6 +57,10 @@ int ovs_nla_get_match(struct sw_flow_match *, const struct nlattr *key,
> int ovs_nla_put_egress_tunnel_key(struct sk_buff *,
> const struct ovs_tunnel_info *);
>
> +int ovs_nla_get_ufid(const struct nlattr *, struct sw_flow_id *, bool log);
> +int ovs_nla_copy_ufid(const struct nlattr *, struct sw_flow_id **, bool log);
> +u32 ovs_nla_get_ufid_flags(const struct nlattr *attr);
> +
> int ovs_nla_copy_actions(const struct nlattr *attr,
> const struct sw_flow_key *key,
> struct sw_flow_actions **sfa, bool log);
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index e0a7fef..7287805 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -85,6 +85,8 @@ struct sw_flow *ovs_flow_alloc(void)
>
> flow->sf_acts = NULL;
> flow->mask = NULL;
> + flow->ufid = NULL;
> + flow->unmasked_key = NULL;
> flow->stats_last_writer = NUMA_NO_NODE;
>
> /* Initialize the default stat node. */
> @@ -139,6 +141,8 @@ static void flow_free(struct sw_flow *flow)
> {
> int node;
>
> + kfree(flow->ufid);
> + kfree(flow->unmasked_key);
> kfree((struct sw_flow_actions __force *)flow->sf_acts);
> for_each_node(node)
> if (flow->stats[node])
> @@ -200,18 +204,28 @@ static struct table_instance *table_instance_alloc(int new_size)
>
> int ovs_flow_tbl_init(struct flow_table *table)
> {
> - struct table_instance *ti;
> + struct table_instance *ti, *ufid_ti;
>
> ti = table_instance_alloc(TBL_MIN_BUCKETS);
>
> if (!ti)
> return -ENOMEM;
>
> + ufid_ti = table_instance_alloc(TBL_MIN_BUCKETS);
> + if (!ufid_ti)
> + goto free_ti;
> +
> rcu_assign_pointer(table->ti, ti);
> + rcu_assign_pointer(table->ufid_ti, ufid_ti);
> INIT_LIST_HEAD(&table->mask_list);
> table->last_rehash = jiffies;
> table->count = 0;
> + table->ufid_count = 0;
> return 0;
> +
> +free_ti:
> + __table_instance_destroy(ti);
> + return -ENOMEM;
> }
>
> static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu)
> @@ -221,13 +235,16 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu)
> __table_instance_destroy(ti);
> }
>
> -static void table_instance_destroy(struct table_instance *ti, bool deferred)
> +static void table_instance_destroy(struct table_instance *ti,
> + struct table_instance *ufid_ti,
> + bool deferred)
> {
> int i;
>
> if (!ti)
> return;
>
> + BUG_ON(!ufid_ti);
> if (ti->keep_flows)
> goto skip_flows;
>
> @@ -236,18 +253,24 @@ static void table_instance_destroy(struct table_instance *ti, bool deferred)
> struct hlist_head *head = flex_array_get(ti->buckets, i);
> struct hlist_node *n;
> int ver = ti->node_ver;
> + int ufid_ver = ufid_ti->node_ver;
>
> - hlist_for_each_entry_safe(flow, n, head, hash_node[ver]) {
> - hlist_del_rcu(&flow->hash_node[ver]);
> + hlist_for_each_entry_safe(flow, n, head, flow_hash.node[ver]) {
> + hlist_del_rcu(&flow->flow_hash.node[ver]);
> + if (flow->ufid)
> + hlist_del_rcu(&flow->ufid_hash.node[ufid_ver]);
> ovs_flow_free(flow, deferred);
> }
> }
>
> skip_flows:
> - if (deferred)
> + if (deferred) {
> call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb);
> - else
> + call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb);
> + } else {
> __table_instance_destroy(ti);
> + __table_instance_destroy(ufid_ti);
> + }
> }
>
> /* No need for locking this function is called from RCU callback or
> @@ -256,8 +279,9 @@ skip_flows:
> void ovs_flow_tbl_destroy(struct flow_table *table)
> {
> struct table_instance *ti = rcu_dereference_raw(table->ti);
> + struct table_instance *ufid_ti = rcu_dereference_raw(table->ufid_ti);
>
> - table_instance_destroy(ti, false);
> + table_instance_destroy(ti, ufid_ti, false);
> }
>
> struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti,
> @@ -272,7 +296,7 @@ struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *ti,
> while (*bucket < ti->n_buckets) {
> i = 0;
> head = flex_array_get(ti->buckets, *bucket);
> - hlist_for_each_entry_rcu(flow, head, hash_node[ver]) {
> + hlist_for_each_entry_rcu(flow, head, flow_hash.node[ver]) {
> if (i < *last) {
> i++;
> continue;
> @@ -294,16 +318,26 @@ static struct hlist_head *find_bucket(struct table_instance *ti, u32 hash)
> (hash & (ti->n_buckets - 1)));
> }
>
> -static void table_instance_insert(struct table_instance *ti, struct sw_flow *flow)
> +static void table_instance_insert(struct table_instance *ti,
> + struct sw_flow *flow)
> +{
> + struct hlist_head *head;
> +
> + head = find_bucket(ti, flow->flow_hash.hash);
> + hlist_add_head_rcu(&flow->flow_hash.node[ti->node_ver], head);
> +}
> +
> +static void ufid_table_instance_insert(struct table_instance *ti,
> + struct sw_flow *flow)
> {
> struct hlist_head *head;
>
> - head = find_bucket(ti, flow->hash);
> - hlist_add_head_rcu(&flow->hash_node[ti->node_ver], head);
> + head = find_bucket(ti, flow->ufid_hash.hash);
> + hlist_add_head_rcu(&flow->ufid_hash.node[ti->node_ver], head);
> }
>
> static void flow_table_copy_flows(struct table_instance *old,
> - struct table_instance *new)
> + struct table_instance *new, bool ufid)
> {
> int old_ver;
> int i;
> @@ -318,15 +352,21 @@ static void flow_table_copy_flows(struct table_instance *old,
>
> head = flex_array_get(old->buckets, i);
>
> - hlist_for_each_entry(flow, head, hash_node[old_ver])
> - table_instance_insert(new, flow);
> + if (ufid)
> + hlist_for_each_entry(flow, head,
> + ufid_hash.node[old_ver])
> + ufid_table_instance_insert(new, flow);
> + else
> + hlist_for_each_entry(flow, head,
> + flow_hash.node[old_ver])
> + table_instance_insert(new, flow);
> }
>
> old->keep_flows = true;
> }
>
> static struct table_instance *table_instance_rehash(struct table_instance *ti,
> - int n_buckets)
> + int n_buckets, bool ufid)
> {
> struct table_instance *new_ti;
>
> @@ -334,27 +374,37 @@ static struct table_instance *table_instance_rehash(struct table_instance *ti,
> if (!new_ti)
> return NULL;
>
> - flow_table_copy_flows(ti, new_ti);
> -
> + flow_table_copy_flows(ti, new_ti, ufid);
> return new_ti;
> }
>
> int ovs_flow_tbl_flush(struct flow_table *flow_table)
> {
> - struct table_instance *old_ti;
> - struct table_instance *new_ti;
> + struct table_instance *old_ti, *new_ti;
> + struct table_instance *old_ufid_ti, *new_ufid_ti;
>
> - old_ti = ovsl_dereference(flow_table->ti);
> new_ti = table_instance_alloc(TBL_MIN_BUCKETS);
> if (!new_ti)
> return -ENOMEM;
> + new_ufid_ti = table_instance_alloc(TBL_MIN_BUCKETS);
> + if (!new_ufid_ti)
> + goto err_free_ti;
> +
> + old_ti = ovsl_dereference(flow_table->ti);
> + old_ufid_ti = ovsl_dereference(flow_table->ufid_ti);
>
> rcu_assign_pointer(flow_table->ti, new_ti);
> + rcu_assign_pointer(flow_table->ufid_ti, new_ufid_ti);
> flow_table->last_rehash = jiffies;
> flow_table->count = 0;
> + flow_table->ufid_count = 0;
>
> - table_instance_destroy(old_ti, true);
> + table_instance_destroy(old_ti, old_ufid_ti, true);
> return 0;
> +
> +err_free_ti:
> + __table_instance_destroy(new_ti);
> + return -ENOMEM;
> }
>
> static u32 flow_hash(const struct sw_flow_key *key, int key_start,
> @@ -407,7 +457,8 @@ bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
> int key_start = flow_key_start(key);
> int key_end = match->range.end;
>
> - return cmp_key(&flow->unmasked_key, key, key_start, key_end);
> + BUG_ON(flow->ufid);
> + return cmp_key(flow->unmasked_key, key, key_start, key_end);
> }
>
> static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
> @@ -424,10 +475,9 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
> ovs_flow_mask_key(&masked_key, unmasked, mask);
> hash = flow_hash(&masked_key, key_start, key_end);
> head = find_bucket(ti, hash);
> - hlist_for_each_entry_rcu(flow, head, hash_node[ti->node_ver]) {
> - if (flow->mask == mask && flow->hash == hash &&
> - flow_cmp_masked_key(flow, &masked_key,
> - key_start, key_end))
> + hlist_for_each_entry_rcu(flow, head, flow_hash.node[ti->node_ver]) {
> + if (flow->mask == mask && flow->flow_hash.hash == hash &&
> + flow_cmp_masked_key(flow, &masked_key, key_start, key_end))
> return flow;
> }
> return NULL;
> @@ -469,7 +519,40 @@ struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
> /* Always called under ovs-mutex. */
> list_for_each_entry(mask, &tbl->mask_list, list) {
> flow = masked_flow_lookup(ti, match->key, mask);
> - if (flow && ovs_flow_cmp_unmasked_key(flow, match)) /* Found */
> + if (flow && !flow->ufid &&
why not NULL check for flow->unmasked_key here rather than ufid?
> + ovs_flow_cmp_unmasked_key(flow, match))
> + return flow;
> + }
> + return NULL;
> +}
> +
> +static u32 ufid_hash(const struct sw_flow_id *sfid)
> +{
> + return arch_fast_hash(sfid->ufid, sfid->ufid_len, 0);
> +}
> +
> +bool ovs_flow_cmp_ufid(const struct sw_flow *flow,
> + const struct sw_flow_id *sfid)
> +{
> + if (flow->ufid->ufid_len != sfid->ufid_len)
> + return false;
> +
> + return !memcmp(flow->ufid->ufid, sfid->ufid, sfid->ufid_len);
> +}
> +
> +struct sw_flow *ovs_flow_tbl_lookup_ufid(struct flow_table *tbl,
> + const struct sw_flow_id *ufid)
> +{
> + struct table_instance *ti = rcu_dereference_ovsl(tbl->ufid_ti);
> + struct sw_flow *flow;
> + struct hlist_head *head;
> + u32 hash;
> +
> + hash = ufid_hash(ufid);
> + head = find_bucket(ti, hash);
> + hlist_for_each_entry_rcu(flow, head, ufid_hash.node[ti->node_ver]) {
> + if (flow->ufid_hash.hash == hash &&
> + ovs_flow_cmp_ufid(flow, ufid))
> return flow;
> }
> return NULL;
> @@ -486,9 +569,10 @@ int ovs_flow_tbl_num_masks(const struct flow_table *table)
> return num;
> }
>
> -static struct table_instance *table_instance_expand(struct table_instance *ti)
> +static struct table_instance *table_instance_expand(struct table_instance *ti,
> + bool ufid)
> {
> - return table_instance_rehash(ti, ti->n_buckets * 2);
> + return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
> }
>
> /* Remove 'mask' from the mask list, if it is not needed any more. */
> @@ -513,10 +597,15 @@ static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask)
> void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow)
> {
> struct table_instance *ti = ovsl_dereference(table->ti);
> + struct table_instance *ufid_ti = ovsl_dereference(table->ufid_ti);
>
> BUG_ON(table->count == 0);
> - hlist_del_rcu(&flow->hash_node[ti->node_ver]);
> + hlist_del_rcu(&flow->flow_hash.node[ti->node_ver]);
> table->count--;
> + if (flow->ufid) {
> + hlist_del_rcu(&flow->ufid_hash.node[ufid_ti->node_ver]);
> + table->ufid_count--;
> + }
>
> /* RCU delete the mask. 'flow->mask' is not NULLed, as it should be
> * accessible as long as the RCU read lock is held.
> @@ -585,34 +674,65 @@ static int flow_mask_insert(struct flow_table *tbl, struct sw_flow *flow,
> }
>
> /* Must be called with OVS mutex held. */
> -int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
> - const struct sw_flow_mask *mask)
> +static void flow_key_insert(struct flow_table *table, struct sw_flow *flow)
> {
> struct table_instance *new_ti = NULL;
> struct table_instance *ti;
> - int err;
>
> - err = flow_mask_insert(table, flow, mask);
> - if (err)
> - return err;
> -
> - flow->hash = flow_hash(&flow->key, flow->mask->range.start,
> - flow->mask->range.end);
> + flow->flow_hash.hash = flow_hash(&flow->key, flow->mask->range.start,
> + flow->mask->range.end);
> ti = ovsl_dereference(table->ti);
> table_instance_insert(ti, flow);
> table->count++;
>
> /* Expand table, if necessary, to make room. */
> if (table->count > ti->n_buckets)
> - new_ti = table_instance_expand(ti);
> + new_ti = table_instance_expand(ti, false);
> else if (time_after(jiffies, table->last_rehash + REHASH_INTERVAL))
> - new_ti = table_instance_rehash(ti, ti->n_buckets);
> + new_ti = table_instance_rehash(ti, ti->n_buckets, false);
>
> if (new_ti) {
> rcu_assign_pointer(table->ti, new_ti);
> - table_instance_destroy(ti, true);
> + call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb);
> table->last_rehash = jiffies;
> }
> +}
> +
> +/* Must be called with OVS mutex held. */
> +static void flow_ufid_insert(struct flow_table *table, struct sw_flow *flow)
> +{
> + struct table_instance *ti;
> +
> + flow->ufid_hash.hash = ufid_hash(flow->ufid);
> + ti = ovsl_dereference(table->ufid_ti);
> + ufid_table_instance_insert(ti, flow);
> + table->ufid_count++;
> +
> + /* Expand table, if necessary, to make room. */
> + if (table->ufid_count > ti->n_buckets) {
> + struct table_instance *new_ti;
> +
> + new_ti = table_instance_expand(ti, true);
> + if (new_ti) {
> + rcu_assign_pointer(table->ufid_ti, new_ti);
> + call_rcu(&ti->rcu, flow_tbl_destroy_rcu_cb);
> + }
> + }
> +}
> +
> +/* Must be called with OVS mutex held. */
> +int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
> + const struct sw_flow_mask *mask)
> +{
> + int err;
> +
> + err = flow_mask_insert(table, flow, mask);
> + if (err)
> + return err;
> + flow_key_insert(table, flow);
> + if (flow->ufid)
> + flow_ufid_insert(table, flow);
> +
> return 0;
> }
>
> diff --git a/net/openvswitch/flow_table.h b/net/openvswitch/flow_table.h
> index 309fa64..454ef92 100644
> --- a/net/openvswitch/flow_table.h
> +++ b/net/openvswitch/flow_table.h
> @@ -47,9 +47,11 @@ struct table_instance {
>
> struct flow_table {
> struct table_instance __rcu *ti;
> + struct table_instance __rcu *ufid_ti;
> struct list_head mask_list;
> unsigned long last_rehash;
> unsigned int count;
> + unsigned int ufid_count;
> };
>
> extern struct kmem_cache *flow_stats_cache;
> @@ -78,8 +80,13 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
> const struct sw_flow_key *);
> struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
> const struct sw_flow_match *match);
> +struct sw_flow *ovs_flow_tbl_lookup_ufid(struct flow_table *,
> + const struct sw_flow_id *);
> +
> bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
> const struct sw_flow_match *match);
> +bool ovs_flow_cmp_ufid(const struct sw_flow *flow,
> + const struct sw_flow_id *sfid);
>
> void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
> const struct sw_flow_mask *mask);
> --
> 1.7.10.4
>
^ permalink raw reply
* Re: [PATCH net-next 0/4] cxgb4/cxgb4vf: T5 BAR2 and ethtool related fixes
From: David Miller @ 2014-12-09 18:32 UTC (permalink / raw)
To: hariprasad; +Cc: netdev, leedom, anish, nirranjan, kumaras
In-Reply-To: <1417615374-12961-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Wed, 3 Dec 2014 19:32:50 +0530
> This series adds new interface to calculate BAR2 SGE queue register address for
> cxgb4 and cxgb4vf driver and some more sge related fixes for T5. Also adds a
> patch which updates the FW version displayed by ethtool after firmware flash.
>
> The patches series is created against 'net-next' tree.
> And includes patches on cxgb4 and cxgb4vf driver.
>
> We have included all the maintainers of respective drivers. Kindly review the
> change and let us know in case of any review comments.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tipc: drop tx side permission checks
From: David Miller @ 2014-12-09 18:30 UTC (permalink / raw)
To: erik.hugne; +Cc: netdev, ying.xue, jon.maloy, richard.alpe, tipc-discussion
In-Reply-To: <1417614284-24125-1-git-send-email-erik.hugne@ericsson.com>
From: <erik.hugne@ericsson.com>
Date: Wed, 3 Dec 2014 14:44:44 +0100
> From: Erik Hugne <erik.hugne@ericsson.com>
>
> Part of the old remote management feature is a piece of code
> that checked permissions on the local system to see if a certain
> operation was permitted, and if so pass the command to a remote
> node. This serves no purpose after the removal of remote management
> with commit 5902385a2440 ("tipc: obsolete the remote management
> feature") so we remove it.
>
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
> Reviewed-by: Ying Xue <ying.xue@windriver.com>
Applied, thanks.
^ permalink raw reply
* Re: [patch net-next v2 2/2] rocker: fix eth_type type in struct rocker_ctrl
From: David Miller @ 2014-12-09 18:29 UTC (permalink / raw)
To: jiri; +Cc: netdev, sfeldma
In-Reply-To: <1417612494-3788-2-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 3 Dec 2014 14:14:54 +0100
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied.
^ permalink raw reply
* Re: [patch net-next v2 1/2] rocker: introduce be put/get variants and use it when appropriate
From: David Miller @ 2014-12-09 18:29 UTC (permalink / raw)
To: jiri; +Cc: netdev, sfeldma
In-Reply-To: <1417612494-3788-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 3 Dec 2014 14:14:53 +0100
> This kills the sparse warnings.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied.
^ permalink raw reply
* Re: [PATCH net] bnx2x: Limit 1G link enforcement
From: David Miller @ 2014-12-09 18:26 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev, Ariel.Elior, Yaniv.Rosner
In-Reply-To: <1417608920-30867-1-git-send-email-Yuval.Mintz@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@qlogic.com>
Date: Wed, 3 Dec 2014 14:15:20 +0200
> From: Yaniv Rosner <Yaniv.Rosner@qlogic.com>
>
> Change 1G-SFP module detection by verifying not only that it's not
> compliant with 10G-Ethernet, but also that it's 1G-ethernet compliant.
>
> Signed-off-by: Yaniv Rosner <Yaniv.Rosner@qlogic.com>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Applied.
^ permalink raw reply
* Re: [PATCH (net.git)] stmmac: fix max coal timer parameter
From: David Miller @ 2014-12-09 18:25 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1417606378-8179-1-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Wed, 3 Dec 2014 12:32:58 +0100
> This patch is to fix the max coalesce timer setting that can be provided
> by ethtool.
> The default value (STMMAC_COAL_TX_TIMER) was used in the set_coalesce helper
> instead of the max one (STMMAC_MAX_COAL_TX_TICK, so defined but not used).
>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: sctp: use MAX_HEADER for headroom reserve in output path
From: David Miller @ 2014-12-09 18:24 UTC (permalink / raw)
To: dborkman; +Cc: linux-sctp, netdev, robert
In-Reply-To: <1417605238-9936-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Wed, 3 Dec 2014 12:13:58 +0100
> To accomodate for enough headroom for tunnels, use MAX_HEADER instead
> of LL_MAX_HEADER. Robert reported that he has hit after roughly 40hrs
> of trinity an skb_under_panic() via SCTP output path (see reference).
> I couldn't reproduce it from here, but not using MAX_HEADER as elsewhere
> in other protocols might be one possible cause for this.
>
> In any case, it looks like accounting on chunks themself seems to look
> good as the skb already passed the SCTP output path and did not hit
> any skb_over_panic(). Given tunneling was enabled in his .config, the
> headroom would have been expanded by MAX_HEADER in this case.
>
> Reported-by: Robert Święcki <robert@swiecki.net>
> Reference: https://lkml.org/lkml/2014/12/1/507
> Fixes: 594ccc14dfe4d ("[SCTP] Replace incorrect use of dev_alloc_skb with alloc_skb in sctp_packet_transmit().")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] cxgb4: Update FW version string to match FW binary version 1.12.25.0
From: David Miller @ 2014-12-09 18:22 UTC (permalink / raw)
To: hariprasad; +Cc: netdev, leedom, anish, nirranjan, kumaras
In-Reply-To: <1417589971-28524-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Wed, 3 Dec 2014 12:29:31 +0530
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCHv2 net] cxgb4: Add a check for flashing FW using ethtool
From: David Miller @ 2014-12-09 18:21 UTC (permalink / raw)
To: hariprasad; +Cc: netdev, leedom, anish, nirranjan, kumaras
In-Reply-To: <1417587590-26248-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Wed, 3 Dec 2014 11:49:50 +0530
> Don't let T4 firmware flash on a T5 adapter and vice-versa
> using ethtool
>
> Based on original work by Casey Leedom <leedom@chelsio.com>
>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
>
> ---
> V2:
> Use bool for return value based on review comment by Sergei Shtylyov
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2] ipv6: remove useless spin_lock/spin_unlock
From: David Miller @ 2014-12-09 18:18 UTC (permalink / raw)
To: duanj.fnst; +Cc: netdev, eric.dumazet
In-Reply-To: <547E7594.7010301@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Wed, 3 Dec 2014 10:29:40 +0800
> xchg is atomic, so there is no necessary to use spin_lock/spin_unlock
> to protect it. At last, remove the redundant
> opt = xchg(&inet6_sk(sk)->opt, opt); statement.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> v2: remove the redundant opt = xchg(&inet6_sk(sk)->opt, opt); statement.
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net v1 0/2] amd-xgbe: AMD XGBE driver fixes 2014-12-02
From: David Miller @ 2014-12-09 18:16 UTC (permalink / raw)
To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20141203001642.17582.32777.stgit@tlendack-t1.amdoffice.net>
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Tue, 2 Dec 2014 18:16:42 -0600
> The following series of patches includes two bug fixes. Unfortunately,
> the first patch will create a conflict when eventually merged into
> net-next but should be very easy to resolve.
>
> - Do not clear the interrupt bit in the xgbe_ring_data structure
> - Associate a Tx SKB with the proper xgbe_ring_data structure
>
> This patch series is based on net.
Series applied, thanks Tom.
^ permalink raw reply
* Re: [PATCH net-next] amd-xgbe: IRQ names require allocated memory
From: David Miller @ 2014-12-09 18:13 UTC (permalink / raw)
To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20141203000718.17370.31628.stgit@tlendack-t1.amdoffice.net>
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Tue, 2 Dec 2014 18:07:18 -0600
> When requesting an irq, the name passed in must be (part of) allocated
> memory. The irq name was a local variable and resulted in random
> characters when listing /proc/interrupts. Add a character field to the
> xgbe_channel structure to hold the irq name and use that.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Applied.
^ permalink raw reply
* Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: David Miller @ 2014-12-09 18:10 UTC (permalink / raw)
To: m-karicheri2-l0cyMroinI0
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, sandeep_n-l0cyMroinI0,
santosh.shilimkar-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1417556503-22290-3-git-send-email-m-karicheri2-l0cyMroinI0@public.gmane.org>
From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
Date: Tue, 2 Dec 2014 16:41:42 -0500
> +static void set_pkt_info_le(u32 buff, u32 buff_len, u32 ndesc,
> + struct knav_dma_desc *desc)
> +{
> + desc->buff_len = cpu_to_le32(buff_len);
> + desc->buff = cpu_to_le32(buff);
> + desc->next_desc = cpu_to_le32(ndesc);
> +}
The members of knav_dma_desc are "u32", so you are going to get tons of
static checker warnings from trying to assign cpu_to_le32()'s result
(which is a le32) into them.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: linux-next 20141208 - net/sched/sch_fq_codel.c:97 suspicious RCU
From: Eric Dumazet @ 2014-12-09 18:07 UTC (permalink / raw)
To: Valdis Kletnieks; +Cc: John Fastabend, David S. Miller, linux-kernel, netdev
In-Reply-To: <6468.1418147500@turing-police.cc.vt.edu>
On Tue, Dec 9, 2014 at 9:51 AM, Valdis Kletnieks
<Valdis.Kletnieks@vt.edu> wrote:
> Spotted this in dmesg while investigating why my wireless broke
> sometime between next-20141201 and next-20141208. Probably not
> related, as wireless has been broken on several boot attempts of -1208,
> but this has popped only once....
>
> Looks like the fault of
>
> commit 46e5da40aec256155cfedee96dd21a75da941f2c
> Author: John Fastabend <john.fastabend@gmail.com>
> Date: Fri Sep 12 20:04:52 2014 -0700
Well, its a (harmless) typo : this should really be an
rcu_dereference_bh() instead of rcu_dereference()
^ permalink raw reply
* Re: [net_test_tools] udpflood: Add IPv6 support
From: David Miller @ 2014-12-09 18:05 UTC (permalink / raw)
To: kafai; +Cc: netdev
In-Reply-To: <1417545706-31249-1-git-send-email-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Tue, 2 Dec 2014 10:41:46 -0800
> This patch:
> 1. Add IPv6 support
> 2. Print timing for every 65536 fib insert operations to observe
> the gc effect (mostly for IPv6 fib).
Applied, thanks.
^ permalink raw reply
* RE: [net-next 05/13] i40e: add range check to i40e_aq_rc_to_posix
From: Nelson, Shannon @ 2014-12-09 17:35 UTC (permalink / raw)
To: Sergei Shtylyov, Kirsher, Jeffrey T, davem@davemloft.net
Cc: netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
jogreene@redhat.com
In-Reply-To: <548705FC.6020805@cogentembedded.com>
> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
> Sent: Tuesday, December 09, 2014 6:24 AM
> To: Kirsher, Jeffrey T; davem@davemloft.net
> Cc: Nelson, Shannon; netdev@vger.kernel.org; nhorman@redhat.com;
> sassmann@redhat.com; jogreene@redhat.com
> Subject: Re: [net-next 05/13] i40e: add range check to
> i40e_aq_rc_to_posix
>
> Hello.
>
> On 12/9/2014 2:22 PM, Jeff Kirsher wrote:
>
> > From: Shannon Nelson <shannon.nelson@intel.com>
>
> > Just to be sure, add a range check to avoid any possible
> > array index-out-of-bound issues.
>
> > Change-ID: I9323bee6732c2a47599816e1d6c6b3a1f8dcbf54
> > Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
> > Acked-by: Michal Kosiarz <michal.kosiarz@intel.com>
> > Tested-by: Jim Young <jamesx.m.young@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 ++
> > drivers/net/ethernet/intel/i40evf/i40e_adminq.h | 2 ++
> > 2 files changed, 4 insertions(+)
>
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> > index 618fe96..4064b1e 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.h
> > @@ -136,6 +136,8 @@ static inline int i40e_aq_rc_to_posix(u16 aq_rc)
> > -EFBIG, /* I40E_AQ_RC_EFBIG */
> > };
> >
> > + if (aq_rc >= (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0])))
>
> There's ARRAY_SIZE() macro exactly for such computations. And parens
> around the last 'aq_to_posix' are not needed.
Yep, you're right. I'll see about getting Jeff to push a v2 of this patch soon.
Thanks,
sln
^ permalink raw reply
* Re: OVS Kernel Datapath development
From: Lori Jakab @ 2014-12-09 18:02 UTC (permalink / raw)
To: Pravin Shelar, netdev,
dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org
In-Reply-To: <CALnjE+rn_Giv+8TM1_E2faBftRkivXMYHcjn-4Eq4Gu15r=CiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/08/2014 06:47 AM, Pravin Shelar wrote:
> Since the beginning OVS kernel datapath development is primarily done
> on external OVS repo. Now we have mostly synced upstream and external
> OVS. So we have decided to change this process. New process is as
> follows.
Patch series that have previous revisions already reviewed with the old
process should now switch to the new process? Or does the change apply
only to new patch series?
-Lori
>
> 1. OVS feature development that involves kernel datapath should be
> done on net-next tree datapath.
> 2. Such feature patch series should be posted on netdev and ovs-dev
> mailing list.
> 3. Once review is done for entire series, kernel and OVS userspace
> patches will be merged in respective repo.
> 4. After the merge developer is suppose to send patches for external
> kernel datapath along with old kernel compatibility code. So that we
> can keep external datapath insync.
>
>
> Thanks,
> Pravin.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH net] tcp: fix more NULL deref after prequeue changes
From: Eric Dumazet @ 2014-12-09 17:56 UTC (permalink / raw)
To: Dann Frazier, David S. Miller; +Cc: netdev
In-Reply-To: <1418145804.27198.0.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
When I cooked commit c3658e8d0f1 ("tcp: fix possible NULL dereference in
tcp_vX_send_reset()") I missed other spots we could deref a NULL
skb_dst(skb)
Again, if a socket is provided, we do not need skb_dst() to get a
pointer to network namespace : sock_net(sk) is good enough.
Reported-by: Dann Frazier <dann.frazier@canonical.com>
Bisected-by: Dann Frazier <dann.frazier@canonical.com>
Tested-by: Dann Frazier <dann.frazier@canonical.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: ca777eff51f7 ("tcp: remove dst refcount false sharing for prequeue mode")
---
net/ipv4/tcp_ipv4.c | 4 ++--
net/ipv6/tcp_ipv6.c | 28 ++++++++++++++--------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 147be2024290..ef7089ca86e2 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -623,6 +623,7 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
arg.iov[0].iov_base = (unsigned char *)&rep;
arg.iov[0].iov_len = sizeof(rep.th);
+ net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
#ifdef CONFIG_TCP_MD5SIG
hash_location = tcp_parse_md5sig_option(th);
if (!sk && hash_location) {
@@ -633,7 +634,7 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
* Incoming packet is checked with md5 hash with finding key,
* no RST generated if md5 hash doesn't match.
*/
- sk1 = __inet_lookup_listener(dev_net(skb_dst(skb)->dev),
+ sk1 = __inet_lookup_listener(net,
&tcp_hashinfo, ip_hdr(skb)->saddr,
th->source, ip_hdr(skb)->daddr,
ntohs(th->source), inet_iif(skb));
@@ -681,7 +682,6 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
if (sk)
arg.bound_dev_if = sk->sk_bound_dev_if;
- net = dev_net(skb_dst(skb)->dev);
arg.tos = ip_hdr(skb)->tos;
ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index dc495ae2ead0..c277951d783b 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -787,16 +787,16 @@ static const struct tcp_request_sock_ops tcp_request_sock_ipv6_ops = {
.queue_hash_add = inet6_csk_reqsk_queue_hash_add,
};
-static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
- u32 tsval, u32 tsecr, int oif,
- struct tcp_md5sig_key *key, int rst, u8 tclass,
- u32 label)
+static void tcp_v6_send_response(struct sock *sk, struct sk_buff *skb, u32 seq,
+ u32 ack, u32 win, u32 tsval, u32 tsecr,
+ int oif, struct tcp_md5sig_key *key, int rst,
+ u8 tclass, u32 label)
{
const struct tcphdr *th = tcp_hdr(skb);
struct tcphdr *t1;
struct sk_buff *buff;
struct flowi6 fl6;
- struct net *net = dev_net(skb_dst(skb)->dev);
+ struct net *net = sk ? sock_net(sk) : dev_net(skb_dst(skb)->dev);
struct sock *ctl_sk = net->ipv6.tcp_sk;
unsigned int tot_len = sizeof(struct tcphdr);
struct dst_entry *dst;
@@ -946,7 +946,7 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb)
(th->doff << 2);
oif = sk ? sk->sk_bound_dev_if : 0;
- tcp_v6_send_response(skb, seq, ack_seq, 0, 0, 0, oif, key, 1, 0, 0);
+ tcp_v6_send_response(sk, skb, seq, ack_seq, 0, 0, 0, oif, key, 1, 0, 0);
#ifdef CONFIG_TCP_MD5SIG
release_sk1:
@@ -957,13 +957,13 @@ release_sk1:
#endif
}
-static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
- u32 win, u32 tsval, u32 tsecr, int oif,
+static void tcp_v6_send_ack(struct sock *sk, struct sk_buff *skb, u32 seq,
+ u32 ack, u32 win, u32 tsval, u32 tsecr, int oif,
struct tcp_md5sig_key *key, u8 tclass,
u32 label)
{
- tcp_v6_send_response(skb, seq, ack, win, tsval, tsecr, oif, key, 0, tclass,
- label);
+ tcp_v6_send_response(sk, skb, seq, ack, win, tsval, tsecr, oif, key, 0,
+ tclass, label);
}
static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb)
@@ -971,7 +971,7 @@ static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb)
struct inet_timewait_sock *tw = inet_twsk(sk);
struct tcp_timewait_sock *tcptw = tcp_twsk(sk);
- tcp_v6_send_ack(skb, tcptw->tw_snd_nxt, tcptw->tw_rcv_nxt,
+ tcp_v6_send_ack(sk, skb, tcptw->tw_snd_nxt, tcptw->tw_rcv_nxt,
tcptw->tw_rcv_wnd >> tw->tw_rcv_wscale,
tcp_time_stamp + tcptw->tw_ts_offset,
tcptw->tw_ts_recent, tw->tw_bound_dev_if, tcp_twsk_md5_key(tcptw),
@@ -986,10 +986,10 @@ static void tcp_v6_reqsk_send_ack(struct sock *sk, struct sk_buff *skb,
/* sk->sk_state == TCP_LISTEN -> for regular TCP_SYN_RECV
* sk->sk_state == TCP_SYN_RECV -> for Fast Open.
*/
- tcp_v6_send_ack(skb, (sk->sk_state == TCP_LISTEN) ?
+ tcp_v6_send_ack(sk, skb, (sk->sk_state == TCP_LISTEN) ?
tcp_rsk(req)->snt_isn + 1 : tcp_sk(sk)->snd_nxt,
- tcp_rsk(req)->rcv_nxt,
- req->rcv_wnd, tcp_time_stamp, req->ts_recent, sk->sk_bound_dev_if,
+ tcp_rsk(req)->rcv_nxt, req->rcv_wnd,
+ tcp_time_stamp, req->ts_recent, sk->sk_bound_dev_if,
tcp_v6_md5_do_lookup(sk, &ipv6_hdr(skb)->daddr),
0, 0);
}
^ permalink raw reply related
* linux-next 20141208 - net/sched/sch_fq_codel.c:97 suspicious RCU
From: Valdis Kletnieks @ 2014-12-09 17:51 UTC (permalink / raw)
To: John Fastabend, Eric Dumazet, David S. Miller; +Cc: linux-kernel, netdev
[-- Attachment #1: Type: text/plain, Size: 3478 bytes --]
Spotted this in dmesg while investigating why my wireless broke
sometime between next-20141201 and next-20141208. Probably not
related, as wireless has been broken on several boot attempts of -1208,
but this has popped only once....
Looks like the fault of
commit 46e5da40aec256155cfedee96dd21a75da941f2c
Author: John Fastabend <john.fastabend@gmail.com>
Date: Fri Sep 12 20:04:52 2014 -0700
net: qdisc: use rcu prefix and silence sparse warnings
but I freely admit that exhausts my RCU debugging skills. :)
[ 104.743212] ===============================
[ 104.743214] [ INFO: suspicious RCU usage. ]
[ 104.743218] 3.18.0-next-20141208 #27 Tainted: G OE
[ 104.743219] -------------------------------
[ 104.743224] net/sched/sch_fq_codel.c:97 suspicious rcu_dereference_check() usage!
[ 104.743227]
other info that might help us debug this:
[ 104.743231]
rcu_scheduler_active = 1, debug_locks = 1
[ 104.743236] 4 locks held by unbound/1873:
[ 104.743239] #0: (rcu_read_lock_bh){......}, at: [<ffffffff8a537066>] rcu_lock_acquire+0x0/0x22
[ 104.743257] #1: (rcu_read_lock_bh){......}, at: [<ffffffff8a4ebd44>] rcu_lock_acquire+0x0/0x22
[ 104.743274] #2: (dev->qdisc_tx_busylock ?: &qdisc_tx_busylock){+.....}, at: [<ffffffff8a4f3ea4>] __dev_queue_xmit+0x32a/0x66a
[ 104.743292] #3: (&qdisc_tx_lock){+.-...}, at: [<ffffffff8a4f3ead>] __dev_queue_xmit+0x333/0x66a
[ 104.743302]
stack backtrace:
[ 104.743307] CPU: 2 PID: 1873 Comm: unbound Tainted: G OE 3.18.0-next-20141208 #27
[ 104.743309] Hardware name: Dell Inc. Latitude E6530/07Y85M, BIOS A15 06/20/2014
[ 104.743312] 0000000000000001 ffff8800b9f3f978 ffffffff8a6ba925 ffff880223bac210
[ 104.743317] 0000000000000001 ffff8800b9f3f9a8 ffffffff8a07f124 ffff8800b9ce4e00
[ 104.743323] ffff880223fea600 0000000000000000 ffff8800c6e2c000 ffff8800b9f3f9f8
[ 104.743329] Call Trace:
[ 104.743336] [<ffffffff8a6ba925>] dump_stack+0x50/0xa8
[ 104.743343] [<ffffffff8a07f124>] lockdep_rcu_suspicious+0xea/0xf3
[ 104.743348] [<ffffffff8a514903>] fq_codel_enqueue+0x79/0x21c
[ 104.743353] [<ffffffff8a6c5228>] ? _raw_spin_lock+0x37/0x3f
[ 104.743357] [<ffffffff8a4f3fb5>] __dev_queue_xmit+0x43b/0x66a
[ 104.743362] [<ffffffff8a4f41ef>] dev_queue_xmit+0xb/0xd
[ 104.743367] [<ffffffff8a537beb>] ip_finish_output2+0x42b/0x46c
[ 104.743371] [<ffffffff8a53838f>] ip_finish_output+0x13c/0x144
[ 104.743376] [<ffffffff8a5393f1>] ip_output+0x4c/0x70
[ 104.743381] [<ffffffff8a538d92>] ip_local_out_sk+0x54/0x5f
[ 104.743385] [<ffffffff8a539c2f>] ip_send_skb+0x12/0x37
[ 104.743390] [<ffffffff8a55d145>] udp_send_skb+0x176/0x1db
[ 104.743394] [<ffffffff8a55d839>] udp_sendmsg+0x63e/0x87c
[ 104.743398] [<ffffffff8a537379>] ? skb_set_owner_w+0x56/0x56
[ 104.743404] [<ffffffff8a09011d>] ? __rcu_read_unlock+0xf5/0xfd
[ 104.743408] [<ffffffff8a568135>] ? rcu_read_unlock+0x3e/0x5d
[ 104.743413] [<ffffffff8a568af9>] inet_sendmsg+0x3d/0x66
[ 104.743419] [<ffffffff8a4d6a4b>] __sock_sendmsg_nosec+0x25/0x27
[ 104.743422] [<ffffffff8a4d80da>] sock_sendmsg+0x5a/0x7b
[ 104.743429] [<ffffffff8a12581b>] ? might_fault+0xae/0xb0
[ 104.743433] [<ffffffff8a4d6c22>] ? audit_sockaddr+0x2b/0x44
[ 104.743437] [<ffffffff8a4da1ff>] SYSC_sendto+0x119/0x15c
[ 104.743443] [<ffffffff8a00e2b5>] ? do_audit_syscall_entry+0xb8/0xc7
[ 104.743447] [<ffffffff8a4da85d>] SyS_sendto+0x9/0xb
[ 104.743453] [<ffffffff8a6c5fd2>] system_call_fastpath+0x12/0x17
[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]
^ permalink raw reply
* [PATCH 1/1] net: dsa: Fix of kernel panic in case of missing PHY.
From: Andrey Volkov @ 2014-12-09 17:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli
Fix of kernel panic in case of missing PHY.
Signed-off-by: Andrey Volkov <andrey.volkov@nexvision.fr>
---
net/dsa/slave.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 528380a..6f89caa 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -512,7 +512,7 @@ static int dsa_slave_fixed_link_update(struct net_device *dev,
}
/* slave device setup *******************************************************/
-static void dsa_slave_phy_setup(struct dsa_slave_priv *p,
+static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
struct net_device *slave_dev)
{
struct dsa_switch *ds = p->parent;
@@ -533,7 +533,7 @@ static void dsa_slave_phy_setup(struct dsa_slave_priv *p,
ret = of_phy_register_fixed_link(port_dn);
if (ret) {
netdev_err(slave_dev, "failed to register fixed PHY\n");
- return;
+ return ret;
}
phy_is_fixed = true;
phy_dn = port_dn;
@@ -555,12 +555,17 @@ static void dsa_slave_phy_setup(struct dsa_slave_priv *p,
*/
if (!p->phy) {
p->phy = ds->slave_mii_bus->phy_map[p->port];
- phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
+ if(p->phy)
+ phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
p->phy_interface);
+ else
+ return -ENODEV;
+
} else {
netdev_info(slave_dev, "attached PHY at address %d [%s]\n",
p->phy->addr, p->phy->drv->name);
}
+ return 0;
}
int dsa_slave_suspend(struct net_device *slave_dev)
@@ -653,7 +658,13 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
p->old_link = -1;
p->old_duplex = -1;
- dsa_slave_phy_setup(p, slave_dev);
+ ret = dsa_slave_phy_setup(p, slave_dev);
+ if (ret) {
+ netdev_err(master, "error %d registering interface %s\n",
+ ret, slave_dev->name);
+ free_netdev(slave_dev);
+ return NULL;
+ }
ret = register_netdev(slave_dev);
if (ret) {
^ permalink raw reply related
* Re: [ovs-dev] OVS Kernel Datapath development
From: Pravin Shelar @ 2014-12-09 17:34 UTC (permalink / raw)
To: thomasfherbert; +Cc: Thomas Graf, dev@openvswitch.org, netdev
In-Reply-To: <54870E56.30305@gmail.com>
On Tue, Dec 9, 2014 at 6:59 AM, Thomas F Herbert
<thomasfherbert@gmail.com> wrote:
> Thanks.
>
> What is the impact on a developer submitting a patch. Do we now submit the
> linux datapath portion of the patch to both netdev and ovs-dev?
>
Yes, if a feature needs kernel datapath changes the feature series
should be posted on both mailing lists.
> --Tom
>
>
> On 12/8/14, 1:30 PM, Pravin Shelar wrote:
>>
>> On Mon, Dec 8, 2014 at 9:15 AM, Thomas Graf <tgraf@noironetworks.com>
>> wrote:
>>>
>>> On 12/07/14 at 08:47pm, Pravin Shelar wrote:
>>>>
>>>> Since the beginning OVS kernel datapath development is primarily done
>>>> on external OVS repo. Now we have mostly synced upstream and external
>>>> OVS. So we have decided to change this process. New process is as
>>>> follows.
>>>>
>>>> 1. OVS feature development that involves kernel datapath should be
>>>> done on net-next tree datapath.
>>>> 2. Such feature patch series should be posted on netdev and ovs-dev
>>>> mailing list.
>>>> 3. Once review is done for entire series, kernel and OVS userspace
>>>> patches will be merged in respective repo.
>>>> 4. After the merge developer is suppose to send patches for external
>>>> kernel datapath along with old kernel compatibility code. So that we
>>>> can keep external datapath insync.
>>>
>>> +1
>>>
>>> Just to be clear, by respective repo do you mean net-next/net or will
>>> you maintain a net-next branch on git.kernel.org and continue doing
>>> pull requests?
>>
>> OVS patches will directly go to net-next/net tree. I am not planning
>> on maintaining any tree on git.kernel.org.
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>
>
> --
> Thomas F. Herbert
>
^ permalink raw reply
* [PATCH net v3 3/6] cxgb4i: additional types of negative advice
From: Karen Xie @ 2014-12-09 17:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v3 3/6] cxgb4i: additional types of negative advice
From: Karen Xie <kxie@chelsio.com>
Treat both CPL_ERR_KEEPALV_NEG_ADVICE and CPL_ERR_PERSIST_NEG_ADVICE as negative advice.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index b834bde..e7a26c0 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -845,6 +845,13 @@ static void csk_act_open_retry_timer(unsigned long data)
}
+static inline bool is_neg_adv(unsigned int status)
+{
+ return status == CPL_ERR_RTX_NEG_ADVICE ||
+ status == CPL_ERR_KEEPALV_NEG_ADVICE ||
+ status == CPL_ERR_PERSIST_NEG_ADVICE;
+}
+
static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
{
struct cxgbi_sock *csk;
@@ -866,7 +873,7 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
"csk 0x%p,%u,0x%lx. ", (&csk->saddr), (&csk->daddr),
atid, tid, status, csk, csk->state, csk->flags);
- if (status == CPL_ERR_RTX_NEG_ADVICE)
+ if (is_neg_adv(status))
goto rel_skb;
module_put(THIS_MODULE);
@@ -972,8 +979,7 @@ static void do_abort_req_rss(struct cxgbi_device *cdev, struct sk_buff *skb)
(&csk->saddr), (&csk->daddr),
csk, csk->state, csk->flags, csk->tid, req->status);
- if (req->status == CPL_ERR_RTX_NEG_ADVICE ||
- req->status == CPL_ERR_PERSIST_NEG_ADVICE)
+ if (is_neg_adv(req->status))
goto rel_skb;
cxgbi_sock_get(csk);
^ permalink raw reply related
* [PATCH net v3 0/6] cxgb4/cxgbi: misc. fixes for cxgb4i
From: Karen Xie @ 2014-12-09 17:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v3 0/6] cxgb4/cxgbi: misc. fixes for cxgb4i
This patch set fixes cxgb4i's tx credit calculation and adds handling of additional rx message and negative advice types. It also removes the duplicate code in cxgb4i to set the outgoing queues of a packet.
Karen Xie (6):
cxgb4i: check if wr header is required when calculating tx credit
cxgb4/cxgb4i: set max. outgoing pdu length in the f/w
cxgb4i: add more types of negative advice
cxgb4i: handle non pdu-aligned rx data
cxgb4i: use cxgb4's set_wr_txq() for setting outgoing queues
libcxgbi: fix the debug print accessing skb after it is freed
Sending to net as the fixes are mostly in the network area and it touches cxgb4's header file (t4fw_api.h).
v2 corrects the "CHECK"s flagged by checkpatch.pl --strict.
v3 splits the 3rd patch from v2 to two seperate patches. Adds detailed commit messages and makes subject more concise. Patch 3/6 also changes the return value of is_neg_adv() from int to bool as recommended by Sergei.
^ permalink raw reply
* [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
From: Karen Xie @ 2014-12-09 17:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v3 1/6] cxgb4i: fix tx credit calculation
From: Karen Xie <kxie@chelsio.com>
- Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
- Any credit related checking should be done before adding the wr header.
- Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 1508125..5c3f15d 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -173,8 +173,12 @@ static int push_tx_frames(struct cxgbi_sock *, int);
*/
static inline int is_ofld_imm(const struct sk_buff *skb)
{
- return skb->len <= (MAX_IMM_TX_PKT_LEN -
- sizeof(struct fw_ofld_tx_data_wr));
+ int length = skb->len;
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ length += sizeof(struct fw_ofld_tx_data_wr);
+
+ return length <= MAX_IMM_TX_PKT_LEN;
}
static void send_act_open_req(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
unsigned int wr_ulp_mode = 0;
- req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
-
if (is_ofld_imm(skb)) {
+ req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
+ sizeof(*req));
req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
FW_WR_COMPL(1) |
FW_WR_IMMDLEN(dlen));
req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
FW_WR_LEN16(credits));
} else {
+ req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
+ sizeof(*req));
req->op_to_immdlen =
cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
FW_WR_COMPL(1) |
@@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
- credits_needed = DIV_ROUND_UP(dlen +
- sizeof(struct fw_ofld_tx_data_wr), 16);
+ credits_needed = DIV_ROUND_UP(dlen, 16);
else
- credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
- + sizeof(struct fw_ofld_tx_data_wr),
- 16);
+ credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
+ 16);
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ credits_needed += DIV_ROUND_UP(
+ sizeof(struct fw_ofld_tx_data_wr), 16);
if (csk->wr_cred < credits_needed) {
log_debug(1 << CXGBI_DBG_PDU_TX,
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 2c7cb1c..aba1af7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -317,8 +317,8 @@ static inline void cxgbi_skcb_clear_flag(struct sk_buff *skb,
__clear_bit(flag, &(cxgbi_skcb_flags(skb)));
}
-static inline int cxgbi_skcb_test_flag(struct sk_buff *skb,
- enum cxgbi_skcb_flags flag)
+static inline int cxgbi_skcb_test_flag(const struct sk_buff *skb,
+ enum cxgbi_skcb_flags flag)
{
return test_bit(flag, &(cxgbi_skcb_flags(skb)));
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox