Netdev List
 help / color / mirror / Atom feed
* 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: [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: [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 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 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: [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] 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: [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.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] 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-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-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] 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 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: [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] tc_act: export uapi header file
From: David Miller @ 2014-12-09 18:34 UTC (permalink / raw)
  To: stephen; +Cc: jiri, jhs, netdev
In-Reply-To: <20141203093317.3c482872@urahara>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 3 Dec 2014 09:33:17 -0800

> This file is used by iproute2 and should be exported.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied.

^ permalink raw reply

* Re: pull request: wireless 2014-12-03
From: David Miller @ 2014-12-09 18:36 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20141203203432.GF2896@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 3 Dec 2014 15:34:33 -0500

> One last(?) batch of fixes hoping to make 3.18...
> 
> In this episode, we have another trio of rtlwifi fixes
> repairing a little more damage from the major update of the
> rtlwifi-family of drivers.  These editing mistakes caused some
> memory corruption and missed a flag critical to proper interrupt
> handling.  Together, these fix the kernel regression reported at
> https://bugzilla.kernel.org/show_bug.cgi?id=88951 by Catalin Iacob.
> 
> Please let me know if there are problems!

Unfortunately, this did not make it for 3.18, but I've pulled it into my
'net' tree which I'll merge into 'net-next' for the merge window.

^ permalink raw reply

* Re: [ovs-dev] OVS Kernel Datapath development
From: Pravin Shelar @ 2014-12-09 18:36 UTC (permalink / raw)
  To: Lori Jakab; +Cc: netdev, dev@openvswitch.org
In-Reply-To: <5487392A.4020409@cisco.com>

On Tue, Dec 9, 2014 at 10:02 AM, Lori Jakab <lojakab@cisco.com> wrote:
> 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?
>

All patch series should follow new process including the one which got
reviewed partially.

> -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.

^ permalink raw reply

* Re: [PATCH net-next v4] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
From: David Miller @ 2014-12-09 18:37 UTC (permalink / raw)
  To: maheshb; +Cc: netdev, edumazet, roopa, makita.toshiaki
In-Reply-To: <1417643184-23440-1-git-send-email-maheshb@google.com>

From: Mahesh Bandewar <maheshb@google.com>
Date: Wed,  3 Dec 2014 13:46:24 -0800

> The commit 56bfa7ee7c ("unregister_netdevice : move RTM_DELLINK to
> until after ndo_uninit") tried to do this ealier but while doing so
> it created a problem. Unfortunately the delayed rtmsg_ifinfo() also
> delayed call to fill_info(). So this translated into asking driver
> to remove private state and then query it's private state. This
> could have catastropic consequences.
> 
> This change breaks the rtmsg_ifinfo() into two parts - one takes the
> precise snapshot of the device by called fill_info() before calling
> the ndo_uninit() and the second part sends the notification using
> collected snapshot.
> 
> It was brought to notice when last link is deleted from an ipvlan device
> when it has free-ed the port and the subsequent .fill_info() call is
> trying to get the info from the port.
...
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Reported-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> Cc: David S. Miller <davem@davemloft.net>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1] net: dsa: Fix of kernel panic in case of missing PHY.
From: Florian Fainelli @ 2014-12-09 18:37 UTC (permalink / raw)
  To: Andrey Volkov, netdev
In-Reply-To: <54873205.30401@nexvision.fr>

On 09/12/14 09:31, Andrey Volkov wrote:
> Fix of kernel panic in case of missing PHY.

Humm, I can trust you, but I would need more context on how you could
trigger a kernel panic here, the only code path that I could imagine is
problematic is the end of dsa_slave_phy_setup():

        if (!p->phy) {
                p->phy = ds->slave_mii_bus->phy_map[p->port];
                phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
                                   p->phy_interface);
        } else {
                netdev_info(slave_dev, "attached PHY at address %d [%s]\n",
                            p->phy->addr, p->phy->drv->name);
        }

Did you observe this with a pure platform_devices/data only setup, or
was that with Device Tree?

Thanks!


> 
> 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

* Re: [PATCH net] Update old iproute2 and Xen Remus links
From: David Miller @ 2014-12-09 18:38 UTC (permalink / raw)
  To: agshew; +Cc: linux-doc, linux-kernel, netdev, corbet, jhs
In-Reply-To: <1417644451-7305-1-git-send-email-agshew@gmail.com>

From: Andrew Shewmaker <agshew@gmail.com>
Date: Wed,  3 Dec 2014 14:07:31 -0800

> Signed-off-by: Andrew Shewmaker <agshew@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: avoid two atomic operations in fast clones
From: David Miller @ 2014-12-09 18:40 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, clm, sd, subramanian.vijay
In-Reply-To: <1417655079.5303.152.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 03 Dec 2014 17:04:39 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Commit ce1a4ea3f125 ("net: avoid one atomic operation in skb_clone()")
> took the wrong way to save one atomic operation.
> 
> It is actually possible to avoid two atomic operations, if we
> do not change skb->fclone values, and only rely on clone_ref
> content to signal if the clone is available or not.
> 
> skb_clone() can simply use the fast clone if clone_ref is 1.
> 
> kfree_skbmem() can avoid the atomic_dec_and_test() if clone_ref is 1.
> 
> Note that because we usually free the clone before the original skb,
> this particular attempt is only done for the original skb to have better
> branch prediction.
> 
> SKB_FCLONE_FREE is removed.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] r8152: redefine REALTEK_USB_DEVICE
From: David Miller @ 2014-12-09 18:41 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-106-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Thu, 4 Dec 2014 10:43:11 +0800

> Redefine REALTEK_USB_DEVICE for the desired USB interface for probe().
> There are three USB interfaces for the device. USB_CLASS_COMM and
> USB_CLASS_CDC_DATA are for ECM mode (config #2). USB_CLASS_VENDOR_SPEC
> is for the vendor mode (config #1). However, we are not interesting
> in USB_CLASS_CDC_DATA for probe(), so redefine REALTEK_USB_DEVICE
> to ignore the USB interface class of USB_CLASS_CDC_DATA.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] tipc: fix missing spinlock init and nullptr oops
From: David Miller @ 2014-12-09 18:42 UTC (permalink / raw)
  To: erik.hugne; +Cc: netdev, tipc-discussion, jon.maloy, ying.xue, richard.alpe
In-Reply-To: <1417622320-19730-1-git-send-email-erik.hugne@ericsson.com>

From: <erik.hugne@ericsson.com>
Date: Wed, 3 Dec 2014 16:58:40 +0100

> From: Erik Hugne <erik.hugne@ericsson.com>
> 
> commit 908344cdda80 ("tipc: fix bug in multicast congestion
> handling") introduced two bugs with the bclink wakeup
> function. This commit fixes the missing spinlock init for the
> waiting_sks list. We also eliminate the race condition
> between the waiting_sks length check/dequeue operations in
> tipc_bclink_wakeup_users by simply removing the redundant
> length check.
> 
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> Acked-by: Tero Aho <Tero.Aho@coriant.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/4 net-next] net: bcmgenet: bcmgenet_init_tx_ring() cleanup
From: David Miller @ 2014-12-09 18:43 UTC (permalink / raw)
  To: pgynther; +Cc: netdev, f.fainelli
In-Reply-To: <20141204041131.830CB2200C7@puck.mtv.corp.google.com>

From: Petri Gynther <pgynther@google.com>
Date: Wed,  3 Dec 2014 20:11:31 -0800 (PST)

> -/* Initialize all house-keeping variables for a TX ring, along
> - * with corresponding hardware registers
> +/*
> + * Initialize a Tx ring along with corresponding hardware registers.
>   */

You are breaking the coding style of this comment.

In the Linux networking, the proper comment layout is:

	/* Like
	 * this.
	 */

not:

	/*
	 * Like this.
	 */

Thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox