Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 4/9] 8021q: use __vlan_hwaccel helpers
From: Michał Mirosław @ 2018-11-08 23:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Ajit Khaparde, Alexey Kuznetsov, bridge,
	Cong Wang, coreteam, Florian Westphal, Hideaki YOSHIFUJI,
	Jamal Hadi Salim, Jiri Pirko, Jozsef Kadlecsik, linux-rdma,
	Mirko Lindner, netfilter-devel, Nikolay Aleksandrov,
	Pablo Neira Ayuso, Roopa Prabhu, Sathya Perla, Somnath Kotur,
	Sriharsha Basavapatna <srih
In-Reply-To: <cover.1541718583.git.mirq-linux@rere.qmqm.pl>

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/8021q/vlan_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4f60e86f4b8d..dd39489c829a 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -57,7 +57,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
 	}
 
 	skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
-	skb->vlan_tci = 0;
+	__vlan_hwaccel_clear_tag(skb);
 
 	rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
 
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 3/9] net/core: use __vlan_hwaccel helpers
From: Michał Mirosław @ 2018-11-08 23:18 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	Ajit Khaparde, Alexey Kuznetsov, bridge, coreteam,
	Florian Westphal, Hideaki YOSHIFUJI, Jozsef Kadlecsik, linux-rdma,
	Mirko Lindner, netfilter-devel, Nikolay Aleksandrov,
	Pablo Neira Ayuso, Roopa Prabhu, Sathya Perla, Somnath Kotur,
	Sriharsha Basavapatna <srih
In-Reply-To: <cover.1541718583.git.mirq-linux@rere.qmqm.pl>

This removes assumptions about VLAN_TAG_PRESENT bit.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/dev.c       | 8 +++++---
 net/core/skbuff.c    | 2 +-
 net/sched/act_vlan.c | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 0ffcbdd55fa9..bf7e0a471186 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4889,7 +4889,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
 		 * and set skb->priority like in vlan_do_receive()
 		 * For the time being, just ignore Priority Code Point
 		 */
-		skb->vlan_tci = 0;
+		__vlan_hwaccel_clear_tag(skb);
 	}
 
 	type = skb->protocol;
@@ -5386,7 +5386,9 @@ static struct list_head *gro_list_prepare(struct napi_struct *napi,
 		}
 
 		diffs = (unsigned long)p->dev ^ (unsigned long)skb->dev;
-		diffs |= p->vlan_tci ^ skb->vlan_tci;
+		diffs |= skb_vlan_tag_present(p) ^ skb_vlan_tag_present(skb);
+		if (skb_vlan_tag_present(p))
+			diffs |= p->vlan_tci ^ skb->vlan_tci;
 		diffs |= skb_metadata_dst_cmp(p, skb);
 		diffs |= skb_metadata_differs(p, skb);
 		if (maclen == ETH_HLEN)
@@ -5652,7 +5654,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
 	__skb_pull(skb, skb_headlen(skb));
 	/* restore the reserve we had after netdev_alloc_skb_ip_align() */
 	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN - skb_headroom(skb));
-	skb->vlan_tci = 0;
+	__vlan_hwaccel_clear_tag(skb);
 	skb->dev = napi->dev;
 	skb->skb_iif = 0;
 	skb->encapsulation = 0;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b4ee5c8b928f..5bb5eb500605 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5123,7 +5123,7 @@ int skb_vlan_pop(struct sk_buff *skb)
 	int err;
 
 	if (likely(skb_vlan_tag_present(skb))) {
-		skb->vlan_tci = 0;
+		__vlan_hwaccel_clear_tag(skb);
 	} else {
 		if (unlikely(!eth_type_vlan(skb->protocol)))
 			return 0;
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index ba677d54a7af..93fdaf707313 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -63,7 +63,7 @@ static int tcf_vlan_act(struct sk_buff *skb, const struct tc_action *a,
 		/* extract existing tag (and guarantee no hw-accel tag) */
 		if (skb_vlan_tag_present(skb)) {
 			tci = skb_vlan_tag_get(skb);
-			skb->vlan_tci = 0;
+			__vlan_hwaccel_clear_tag(skb);
 		} else {
 			/* in-payload vlan tag, pop it */
 			err = __skb_vlan_pop(skb, &tci);
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 2/9] nfnetlink/queue: use __vlan_hwaccel helpers
From: Michał Mirosław @ 2018-11-08 23:18 UTC (permalink / raw)
  To: netdev
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	netfilter-devel, coreteam, Ajit Khaparde, Alexey Kuznetsov,
	bridge, Cong Wang, David S. Miller, Hideaki YOSHIFUJI,
	Jamal Hadi Salim, Jiri Pirko, linux-rdma, Mirko Lindner,
	Nikolay Aleksandrov, Roopa Prabhu, Sathya Perla, Somnath Kotur,
	Sriharsha Basavapatna <srih
In-Reply-To: <cover.1541718583.git.mirq-linux@rere.qmqm.pl>

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/netfilter/nfnetlink_queue.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 43041f087eb3..1ce30efe6854 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1148,8 +1148,9 @@ static int nfqa_parse_bridge(struct nf_queue_entry *entry,
 		if (!tb[NFQA_VLAN_TCI] || !tb[NFQA_VLAN_PROTO])
 			return -EINVAL;
 
-		entry->skb->vlan_tci = ntohs(nla_get_be16(tb[NFQA_VLAN_TCI]));
-		entry->skb->vlan_proto = nla_get_be16(tb[NFQA_VLAN_PROTO]);
+		__vlan_hwaccel_put_tag(entry->skb,
+			nla_get_be16(tb[NFQA_VLAN_PROTO]),
+			ntohs(nla_get_be16(tb[NFQA_VLAN_TCI])));
 	}
 
 	if (nfqa[NFQA_L2HDR]) {
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 1/9] cxgb4: use __vlan_hwaccel helpers
From: Michał Mirosław @ 2018-11-08 23:18 UTC (permalink / raw)
  To: netdev
  Cc: Steve Wise, linux-rdma, Ajit Khaparde, Alexey Kuznetsov, bridge,
	Cong Wang, coreteam, David S. Miller, Florian Westphal,
	Hideaki YOSHIFUJI, Jamal Hadi Salim, Jiri Pirko, Jozsef Kadlecsik,
	Mirko Lindner, netfilter-devel, Nikolay Aleksandrov,
	Pablo Neira Ayuso, Roopa Prabhu, Sathya Perla, Somnath Kotur
In-Reply-To: <cover.1541718583.git.mirq-linux@rere.qmqm.pl>

Use __vlan_hwaccel_put_tag() to set vlan tag and proto fields.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/infiniband/hw/cxgb4/cm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 615413bd3e8d..8ed01e07c463 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -3944,7 +3944,7 @@ static int rx_pkt(struct c4iw_dev *dev, struct sk_buff *skb)
 	} else {
 		vlan_eh = (struct vlan_ethhdr *)(req + 1);
 		iph = (struct iphdr *)(vlan_eh + 1);
-		skb->vlan_tci = ntohs(cpl->vlan);
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(cpl->vlan));
 	}
 
 	if (iph->version != 0x4)
-- 
2.19.1

^ permalink raw reply related

* [PATCH net-next 0/9] Use __vlan_hwaccel_*() helpers
From: Michał Mirosław @ 2018-11-08 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Ajit Khaparde, Alexey Kuznetsov, bridge, Cong Wang, coreteam,
	David S. Miller, Florian Westphal, Hideaki YOSHIFUJI,
	Jamal Hadi Salim, Jiri Pirko, Jozsef Kadlecsik, linux-rdma,
	Mirko Lindner, netfilter-devel, Nikolay Aleksandrov,
	Pablo Neira Ayuso, Roopa Prabhu, Sathya Perla, Somnath Kotur,
	Sriharsha Basavapatna <srih

This series removes from networking core and driver code an assumption
about how VLAN tag presence is stored in an skb. This will allow to free
up overloading of VLAN.CFI bit to incidate tag's presence.

Michał Mirosław (9):
  cxgb4: use __vlan_hwaccel helpers
  nfnetlink/queue: use __vlan_hwaccel helpers
  net/core: use __vlan_hwaccel helpers
  8021q: use __vlan_hwaccel helpers
  bridge: use __vlan_hwaccel helpers
  ipv4/tunnel: use __vlan_hwaccel helpers
  benet: use __vlan_hwaccel helpers
  mlx4: use __vlan_hwaccel helpers
  sky2: use __vlan_hwaccel helpers

 drivers/infiniband/hw/cxgb4/cm.c            |  2 +-
 drivers/net/ethernet/emulex/benet/be_main.c | 13 +++++++++----
 drivers/net/ethernet/marvell/sky2.c         |  6 ++----
 drivers/net/ethernet/mellanox/mlx4/en_rx.c  |  2 +-
 net/8021q/vlan_core.c                       |  2 +-
 net/bridge/br_netfilter_hooks.c             | 15 +++++++++------
 net/bridge/br_private.h                     |  2 +-
 net/bridge/br_vlan.c                        |  6 +++---
 net/core/dev.c                              |  8 +++++---
 net/core/skbuff.c                           |  2 +-
 net/ipv4/ip_tunnel_core.c                   |  2 +-
 net/netfilter/nfnetlink_queue.c             |  5 +++--
 net/sched/act_vlan.c                        |  2 +-
 13 files changed, 38 insertions(+), 29 deletions(-)

-- 
2.19.1

^ permalink raw reply

* Re: [net-next PATCH v2] net: sched: cls_flower: Classify packets using port ranges
From: David Miller @ 2018-11-08 23:15 UTC (permalink / raw)
  To: amritha.nambiar
  Cc: netdev, jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong,
	jiri
In-Reply-To: <154162576240.57813.14471543870620442657.stgit@anamhost.jf.intel.com>

From: Amritha Nambiar <amritha.nambiar@intel.com>
Date: Wed, 07 Nov 2018 13:22:42 -0800

> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 401d0c1..b63c3cf 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -405,6 +405,11 @@ enum {
>  	TCA_FLOWER_KEY_UDP_SRC,		/* be16 */
>  	TCA_FLOWER_KEY_UDP_DST,		/* be16 */
>  
> +	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
> +	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
> +	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
> +	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
> +
>  	TCA_FLOWER_FLAGS,
>  	TCA_FLOWER_KEY_VLAN_ID,		/* be16 */
>  	TCA_FLOWER_KEY_VLAN_PRIO,	/* u8   */
> @@ -518,6 +523,8 @@ enum {

I don't think you can do this without breaking UAPI, this changes the
value of TCA_FLOWER_FLAGS and all subsequent values in this
enumeration.

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: use phy_id_mask value zero for exact match
From: Florian Fainelli @ 2018-11-08 23:06 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <20181108205349.GG5259@lunn.ch>

On 11/8/18 12:53 PM, Andrew Lunn wrote:
>>> Maybe we can find a clever way with a macro to specify only the PHY OUI
>>> and compute a suitable mask automatically?
>>>
>> I don't think so. For Realtek each driver is specific even to a model
>> revision (therefore mask 0xffffffff). Same applies to intel-xway.
>> In broadcom.c we have masks 0xfffffff0, so for each model, independent
>> of revision number. There is no general rule.
>> Also we can't simply check for the first-bit-set to derive a mask.
> 
> I'm crystal ball gazing, but i think Florian was meaning something like
> 
> #define PHY_ID_UNIQUE(_id) \
> 	.phy_id = _id_; \
> 	.phy_id_mask = 0xffffffff;
> 
> It is the boilerplate setting .phy_id_mask which you don't like. This removes that boilerplate.

Your crystal ball gazing skills are good, that is what I meant, we could
also define another macro which does not match the revision bits, and
that would likely cover everything that is already out there.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: phy: use phy_id_mask value zero for exact match
From: David Miller @ 2018-11-08 23:05 UTC (permalink / raw)
  To: hkallweit1; +Cc: f.fainelli, andrew, netdev
In-Reply-To: <08d52675-f308-4ebb-4ec4-f6c7ac0b6b06@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 7 Nov 2018 21:52:31 +0100

> A phy_id_mask value zero means every PHYID matches, therefore
> value zero isn't used. So we can safely redefine the semantics
> of value zero to mean "exact match". This allows to avoid some
> boilerplate code in PHY driver configs.
> 
> Realtek PHY driver is the first user of this change.

It looks like there will be some changes to this series, so I will
wait for the next version.

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: phy: improve and simplify phylib state machine
From: David Miller @ 2018-11-08 23:04 UTC (permalink / raw)
  To: f.fainelli; +Cc: hkallweit1, andrew, netdev
In-Reply-To: <911ca9fa-e994-d3f3-2e8d-6f16631c1757@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 8 Nov 2018 15:00:01 -0800

> On 11/8/18 2:58 PM, David Miller wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>> Date: Wed, 7 Nov 2018 20:41:52 +0100
>> 
>>> This patch series is based on two axioms:
>>>
>>> - During autoneg a PHY always reports the link being down
>>>
>>> - Info in clause 22/45 registers doesn't allow to differentiate between
>>>   these two states:
>>>   1. Link is physically down
>>>   2. A link partner is connected and PHY is autonegotiating
>>>   In both cases "link up" and "aneg finished" bits aren't set.
>>>   One consequence is that having separate states PHY_NOLINK and PHY_AN
>>>   isn't needed.
>>>
>>> By using these two axioms the state machine can be significantly
>>> simplified.
>> 
>> So how are we going to move forward on this?
>> 
>> Maybe we can apply this series and just watch carefully for any
>> problems that get reported or are found?
> 
> Given Heiner is always responsive and taking care of fixing what might
> be/have broken, no objections with me on that.

Great, I've applied this series to net-next then.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: phy: improve and simplify phylib state machine
From: Andrew Lunn @ 2018-11-08 23:01 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, hkallweit1, netdev
In-Reply-To: <911ca9fa-e994-d3f3-2e8d-6f16631c1757@gmail.com>

On Thu, Nov 08, 2018 at 03:00:01PM -0800, Florian Fainelli wrote:
> On 11/8/18 2:58 PM, David Miller wrote:
> > From: Heiner Kallweit <hkallweit1@gmail.com>
> > Date: Wed, 7 Nov 2018 20:41:52 +0100
> > 
> >> This patch series is based on two axioms:
> >>
> >> - During autoneg a PHY always reports the link being down
> >>
> >> - Info in clause 22/45 registers doesn't allow to differentiate between
> >>   these two states:
> >>   1. Link is physically down
> >>   2. A link partner is connected and PHY is autonegotiating
> >>   In both cases "link up" and "aneg finished" bits aren't set.
> >>   One consequence is that having separate states PHY_NOLINK and PHY_AN
> >>   isn't needed.
> >>
> >> By using these two axioms the state machine can be significantly
> >> simplified.
> > 
> > So how are we going to move forward on this?
> > 
> > Maybe we can apply this series and just watch carefully for any
> > problems that get reported or are found?
> 
> Given Heiner is always responsive and taking care of fixing what might
> be/have broken, no objections with me on that.

Yes, lets try it.

     Andrew

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: phy: improve and simplify phylib state machine
From: Florian Fainelli @ 2018-11-08 23:00 UTC (permalink / raw)
  To: David Miller, hkallweit1; +Cc: andrew, netdev
In-Reply-To: <20181108.145846.295661679780466934.davem@davemloft.net>

On 11/8/18 2:58 PM, David Miller wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Wed, 7 Nov 2018 20:41:52 +0100
> 
>> This patch series is based on two axioms:
>>
>> - During autoneg a PHY always reports the link being down
>>
>> - Info in clause 22/45 registers doesn't allow to differentiate between
>>   these two states:
>>   1. Link is physically down
>>   2. A link partner is connected and PHY is autonegotiating
>>   In both cases "link up" and "aneg finished" bits aren't set.
>>   One consequence is that having separate states PHY_NOLINK and PHY_AN
>>   isn't needed.
>>
>> By using these two axioms the state machine can be significantly
>> simplified.
> 
> So how are we going to move forward on this?
> 
> Maybe we can apply this series and just watch carefully for any
> problems that get reported or are found?

Given Heiner is always responsive and taking care of fixing what might
be/have broken, no objections with me on that.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: phy: improve and simplify phylib state machine
From: David Miller @ 2018-11-08 22:58 UTC (permalink / raw)
  To: hkallweit1; +Cc: f.fainelli, andrew, netdev
In-Reply-To: <922c223b-7bc0-e0ec-345d-2034b796af91@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 7 Nov 2018 20:41:52 +0100

> This patch series is based on two axioms:
> 
> - During autoneg a PHY always reports the link being down
> 
> - Info in clause 22/45 registers doesn't allow to differentiate between
>   these two states:
>   1. Link is physically down
>   2. A link partner is connected and PHY is autonegotiating
>   In both cases "link up" and "aneg finished" bits aren't set.
>   One consequence is that having separate states PHY_NOLINK and PHY_AN
>   isn't needed.
> 
> By using these two axioms the state machine can be significantly
> simplified.

So how are we going to move forward on this?

Maybe we can apply this series and just watch carefully for any
problems that get reported or are found?

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Edward Cree @ 2018-11-08 22:56 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Martin KaFai Lau, Yonghong Song, Alexei Starovoitov,
	Daniel Borkmann, Network Development, Kernel Team
In-Reply-To: <CAADnVQJ8Erh=-3Cx1zEhPartsvNz2f-MX-obYDDqAicOyg4nWg@mail.gmail.com>

On 08/11/18 19:42, Alexei Starovoitov wrote:
> same link let's continue at 1pm PST. 
So, one thing we didn't really get onto was maps, and you mentioned that it
 wasn't really clear what I was proposing there.
What I have in mind comes in two parts:
1) map type.  A new BTF_KIND_MAP with metadata 'key_type', 'value_type'
 (both are type_ids referencing other BTF type records), describing the
 type "map from key_type to value_type".
2) record in the 'instances' table.  This would have a name_off (the
 name of the map), a type_id (pointing at a BTF_KIND_MAP in the 'types'
 table), and potentially also some indication of what symbol (from
 section 'maps') refers to this map.  This is pretty much the exact
 same metadata that a function in the 'instances' table has, the only
 differences being
 (a) function's type_id points at a BTF_KIND_FUNC record
 (b) function's symbol indication refers from .text section
 (c) in future functions may be nested inside other functions, whereas
 AIUI a map can't live inside a function.  (But a variable, which is
 the other thing that would want to go in an 'instances' table, can.)
So the 'instances' table record structure looks like

struct btf_instance {
    __u32 type_id; /* Type of object declared.  An index into type section */
    __u32 name_off; /* Name of object.  An offset into string section */
    __u32 parent; /* Containing object if any (else 0).  An index into instance section */
};

and we extend the BTF header:

struct btf_header {
    __u16   magic;
    __u8    version;
    __u8    flags;
    __u32   hdr_len;

    /* All offsets are in bytes relative to the end of this header */
    __u32   type_off;      /* offset of type section       */
    __u32   type_len;      /* length of type section       */
    __u32   str_off;       /* offset of string section     */
    __u32   str_len;       /* length of string section     */
    __u32   inst_off;      /* offset of instance section   */
    __u32   inst_len;      /* length of instance section   */
};

Then in the .BTF.ext section, we have both

struct bpf_func_info {
    __u32 prog_symbol; /* Index of symbol giving address of subprog */
    __u32 inst_id; /* Index into instance section */
}

struct bpf_map_info {
{
    __u32 map_symbol; /* Index of symbol creating this map */
    __u32 inst_id; /* Index into instance section */
}

(either living in different subsections, or in a single table with
 the addition of a kind field, or in a single table relying on the
 ultimately referenced type to distinguish funcs from maps).

Note that the name (in btf_instance) of a map or function need not
 match the name of the corresponding symbol; we use the .BTF.ext
 section to tie together btf_instance IDs and symbol IDs.  Then in
 the case of functions (subprogs), the prog_symbol can be looked
 up in the ELF symbol table to find the address (== insn_offset)
 of the subprog, as well as the section containing it (since that
 might not be .text).  Similarly in the case of maps the BTF info
 about the map is connected with the info in the maps section.

Now when the loader has munged this, what it passes to the kernel
 might not have map_symbol, but instead map_fd.  Instead of
 prog_symbol it will have whatever identifies the subprog in the
 blob of stuff it feeds to the kernel (so probably insn_offset).

All this would of course require a bit more compiler support than
 the current BPF_ANNOTATE_KV_PAIR, since that just causes the
 existing BTF machinery to declare a specially constructed struct
 type.  At the C level you could still have BPF_ANNOTATE_KV_PAIR
 and the '____bpf_map_foo' name, but then the compiler would
 recognise that and convert it into an instance record by looking
 up the name 'foo' in its "maps" section.  That way the special
 ____bpf_map_* handling (which ties map names to symbol names,
 also) would be entirely compiler-internal and not 'leak out' into
 the definition of the format.  Frontends for other languages
 which do possess a native map type (e.g. Python dict) might have
 other ways of indicating the key/value type of a map at source
 level (e.g. PEP 484) and could directly generate the appropriate
 BTF_KIND_MAP and bpf_map_info records rather than (as they would
 with the current design) having to encode the information as a
 struct ____bpf_map_foo type-definition.


While I realise the desire to concentrate on one topic at once, I
 think this question of maps should be discussed in tomorrow's
 call, since it is when we start having other kinds of instances
 besides functions that the advantages of my design become
 apparent, unifying the process of 'declaration' of functions,
 maps, and (eventually) variables while separating them all from
 the process of 'definition' of the types of all three.

Thank you for your continued patience with me.
-Ed

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Heiner Kallweit @ 2018-11-08 22:40 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <96112482-1293-0276-8bcc-44bf1beabd59@gmail.com>

On 08.11.2018 23:33, Florian Fainelli wrote:
> On 11/8/18 1:55 PM, Heiner Kallweit wrote:
>> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think
>> using interrupts isn't possible if a driver defines neither
>> config_intr nor ack_interrupts callback. So we can replace checking
>> flag PHY_HAS_INTERRUPT with checking for these callbacks.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/phy/phy_device.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index d165a2c82..33e51b955 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -2104,8 +2104,9 @@ static int phy_probe(struct device *dev)
>>  	/* Disable the interrupt if the PHY doesn't support it
>>  	 * but the interrupt is still a valid one
>>  	 */
>> -	if (!(phydrv->flags & PHY_HAS_INTERRUPT) &&
>> -	    phy_interrupt_is_valid(phydev))
>> +	 if (!phydrv->config_intr &&
>> +	     !phydrv->ack_interrupt &&
>> +	     phy_interrupt_is_valid(phydev))
>>  		phydev->irq = PHY_POLL;
> 
> I would introduce an inline helper function which checks for
> drv->config_intr and config_ack_interrupt, that way if we ever have to
> introduce an additional function in the future, we just update the
> helper to check for that.
> 
> Other than that, LGTM
> 
OK, will add a helper and remove PHY_HAS_INTERRUPT from all drivers.

^ permalink raw reply

* [PATCH net] net: mvneta: Don't advertise 2.5G modes
From: Maxime Chevallier @ 2018-11-09  8:17 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
	thomas.petazzoni, gregory.clement, Andrew Lunn, Russell King,
	linux-arm-kernel

Using 2.5G speed relies on the SerDes lanes being configured
accordingly. The lanes have to be reconfigured to switch between
1G and 2.5G, and for now only the bootloader does this configuration.

In the case we add a Comphy driver to handle switching the lanes
dynamically, it's better for now to stick with supporting only 1G and
add advertisement for 2.5G once we really are capable of handling both
speeds without problem.

Since the interface mode is initialy taken from the DT, we want to make
sure that adding comphy support won't break boards that don't update
their dtb.

Fixes: da58a931f248 ("net: mvneta: Add support for 2500Mbps SGMII")
Reported-by: Andrew Lunn <andrew@lunn.ch>
Reported-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 5bfd349bf41a..c19ecd153499 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3343,7 +3343,6 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	if (state->interface != PHY_INTERFACE_MODE_NA &&
 	    state->interface != PHY_INTERFACE_MODE_QSGMII &&
 	    state->interface != PHY_INTERFACE_MODE_SGMII &&
-	    state->interface != PHY_INTERFACE_MODE_2500BASEX &&
 	    !phy_interface_mode_is_8023z(state->interface) &&
 	    !phy_interface_mode_is_rgmii(state->interface)) {
 		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
@@ -3357,14 +3356,9 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	/* Asymmetric pause is unsupported */
 	phylink_set(mask, Pause);
 
-	/* We cannot use 1Gbps when using the 2.5G interface. */
-	if (state->interface == PHY_INTERFACE_MODE_2500BASEX) {
-		phylink_set(mask, 2500baseT_Full);
-		phylink_set(mask, 2500baseX_Full);
-	} else {
-		phylink_set(mask, 1000baseT_Full);
-		phylink_set(mask, 1000baseX_Full);
-	}
+	/* Half-duplex at speeds higher than 100Mbit is unsupported */
+	phylink_set(mask, 1000baseT_Full);
+	phylink_set(mask, 1000baseX_Full);
 
 	if (!phy_interface_mode_is_8023z(state->interface)) {
 		/* 10M and 100M are only supported in non-802.3z mode */
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next] net: phy: improve struct phy_device member interrupts handling
From: Heiner Kallweit @ 2018-11-08 22:39 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20181108222436.GH5259@lunn.ch>

On 08.11.2018 23:24, Andrew Lunn wrote:
> On Thu, Nov 08, 2018 at 10:36:33PM +0100, Heiner Kallweit wrote:
>> As a heritage from the very early days of phylib member interrupts is
>> defined as u32 even though it's just a flag whether interrupts are
>> enabled. So we can change it to a bitfield member. In addition change
>> the code dealing with this member in a way that it's clear we're
>> dealing with a bool value.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> Actually this member isn't needed at all and could be replaced with
>> a parameter in phy_driver->config_intr. But this would mean an API
>> change, maybe I come up with a proposal later.
>> ---
>>  drivers/net/phy/phy.c |  2 +-
>>  include/linux/phy.h   | 10 +++++-----
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index dd5bff955..a5e6acfe6 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -115,7 +115,7 @@ static int phy_clear_interrupt(struct phy_device *phydev)
>>   *
>>   * Returns 0 on success or < 0 on error.
>>   */
>> -static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
>> +static int phy_config_interrupt(struct phy_device *phydev, bool interrupts)
>>  {
>>  	phydev->interrupts = interrupts;
>>  	if (phydev->drv->config_intr)
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 5dd85c441..fc90af152 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -263,8 +263,8 @@ static inline struct mii_bus *devm_mdiobus_alloc(struct device *dev)
>>  void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
>>  struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
>>  
>> -#define PHY_INTERRUPT_DISABLED	0x0
>> -#define PHY_INTERRUPT_ENABLED	0x80000000
>> +#define PHY_INTERRUPT_DISABLED	0
>> +#define PHY_INTERRUPT_ENABLED	1
> 
> Hi Heiner
> 
> Since this is passed around as a bool, false/true would be better than
> 0/1.
> 
I thought about that before submitting the patch. I preferred to go with
0/1 because we assign this value to a 1 bit bitfield member which supports
values 0/1 only. So neither option is perfect IMO.

> 	Andrew
> 
Heiner

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Florian Fainelli @ 2018-11-08 22:34 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <20181108223028.GI5259@lunn.ch>

On 11/8/18 2:30 PM, Andrew Lunn wrote:
> On Thu, Nov 08, 2018 at 10:54:50PM +0100, Heiner Kallweit wrote:
>> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think
>> using interrupts isn't possible if a driver defines neither
>> config_intr nor ack_interrupts callback. So we can replace checking
>> flag PHY_HAS_INTERRUPT with checking for these callbacks.
>>
>> This allows to remove this flag from a lot of driver configs, let's
>> start with the Realtek driver.
> 
> Hi Heiner
> 
> Ideally, we should do this to all the drivers, not just one. If we
> leave PHY_HAS_INTERRUPT, people are going to use it. That then makes
> the realtek driver then different to all the other drivers, and at
> some point somebody will do something which breaks it because they
> don't know the realtek driver is special.

Agreed.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Florian Fainelli @ 2018-11-08 22:33 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <1ad7d0ef-7e7e-93b2-3b3c-3c4704b3b0cc@gmail.com>

On 11/8/18 1:55 PM, Heiner Kallweit wrote:
> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think
> using interrupts isn't possible if a driver defines neither
> config_intr nor ack_interrupts callback. So we can replace checking
> flag PHY_HAS_INTERRUPT with checking for these callbacks.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phy_device.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index d165a2c82..33e51b955 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2104,8 +2104,9 @@ static int phy_probe(struct device *dev)
>  	/* Disable the interrupt if the PHY doesn't support it
>  	 * but the interrupt is still a valid one
>  	 */
> -	if (!(phydrv->flags & PHY_HAS_INTERRUPT) &&
> -	    phy_interrupt_is_valid(phydev))
> +	 if (!phydrv->config_intr &&
> +	     !phydrv->ack_interrupt &&
> +	     phy_interrupt_is_valid(phydev))
>  		phydev->irq = PHY_POLL;

I would introduce an inline helper function which checks for
drv->config_intr and config_ack_interrupt, that way if we ever have to
introduce an additional function in the future, we just update the
helper to check for that.

Other than that, LGTM
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Andrew Lunn @ 2018-11-08 22:30 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <193e3970-8ce2-1221-357a-7b7f9f6aea76@gmail.com>

On Thu, Nov 08, 2018 at 10:54:50PM +0100, Heiner Kallweit wrote:
> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think
> using interrupts isn't possible if a driver defines neither
> config_intr nor ack_interrupts callback. So we can replace checking
> flag PHY_HAS_INTERRUPT with checking for these callbacks.
> 
> This allows to remove this flag from a lot of driver configs, let's
> start with the Realtek driver.

Hi Heiner

Ideally, we should do this to all the drivers, not just one. If we
leave PHY_HAS_INTERRUPT, people are going to use it. That then makes
the realtek driver then different to all the other drivers, and at
some point somebody will do something which breaks it because they
don't know the realtek driver is special.

	Andrew

^ permalink raw reply

* Re: [PATCH net-next] net: phy: improve struct phy_device member interrupts handling
From: Andrew Lunn @ 2018-11-08 22:24 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <44805a11-3063-4f1f-2fd2-b268155d4935@gmail.com>

On Thu, Nov 08, 2018 at 10:36:33PM +0100, Heiner Kallweit wrote:
> As a heritage from the very early days of phylib member interrupts is
> defined as u32 even though it's just a flag whether interrupts are
> enabled. So we can change it to a bitfield member. In addition change
> the code dealing with this member in a way that it's clear we're
> dealing with a bool value.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> Actually this member isn't needed at all and could be replaced with
> a parameter in phy_driver->config_intr. But this would mean an API
> change, maybe I come up with a proposal later.
> ---
>  drivers/net/phy/phy.c |  2 +-
>  include/linux/phy.h   | 10 +++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index dd5bff955..a5e6acfe6 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -115,7 +115,7 @@ static int phy_clear_interrupt(struct phy_device *phydev)
>   *
>   * Returns 0 on success or < 0 on error.
>   */
> -static int phy_config_interrupt(struct phy_device *phydev, u32 interrupts)
> +static int phy_config_interrupt(struct phy_device *phydev, bool interrupts)
>  {
>  	phydev->interrupts = interrupts;
>  	if (phydev->drv->config_intr)
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5dd85c441..fc90af152 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -263,8 +263,8 @@ static inline struct mii_bus *devm_mdiobus_alloc(struct device *dev)
>  void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);
>  struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
>  
> -#define PHY_INTERRUPT_DISABLED	0x0
> -#define PHY_INTERRUPT_ENABLED	0x80000000
> +#define PHY_INTERRUPT_DISABLED	0
> +#define PHY_INTERRUPT_ENABLED	1

Hi Heiner

Since this is passed around as a bool, false/true would be better than
0/1.

	Andrew

^ permalink raw reply

* Re: [PATCH 04/20] octeontx2-af: NPC MCAM entry alloc/free support
From: David Miller @ 2018-11-08 22:22 UTC (permalink / raw)
  To: sunil.kovvuri; +Cc: netdev, arnd, linux-soc, sgoutham
In-Reply-To: <1541702161-30673-5-git-send-email-sunil.kovvuri@gmail.com>

From: sunil.kovvuri@gmail.com
Date: Fri,  9 Nov 2018 00:05:45 +0530

> +int rvu_mbox_handler_NPC_MCAM_ALLOC_ENTRY(struct rvu *rvu,
> +					  struct npc_mcam_alloc_entry_req *req,
> +					  struct npc_mcam_alloc_entry_rsp *rsp);
> +int rvu_mbox_handler_NPC_MCAM_FREE_ENTRY(struct rvu *rvu,
> +					 struct npc_mcam_free_entry_req *req,
> +					 struct msg_rsp *rsp);

Please don't use capitalized letters or StUdLyCaPs for function and variable
names.

Keep capitalized letters for CPP macros.

This is how programmers visually can see if something is a real C declaration
or some CPP stuff.

^ permalink raw reply

* Re: [Patch net] ip: fix csum_sub() with csum_block_sub()
From: Cong Wang @ 2018-11-08 22:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Dimitris Michailidis, Linux Kernel Network Developers,
	Paolo Abeni, Willem de Bruijn
In-Reply-To: <CANn89iLqG5bEsJPfeNsm51P6UEkJOu4j0hFFODTHb4W+vqAe=w@mail.gmail.com>

On Thu, Nov 8, 2018 at 2:17 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Nov 8, 2018 at 2:04 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > When subtracting the checksum of a block of data,
> > csum_block_sub() must be used to respect the offset.
> >
> > We learned this lesson from both commit d55bef5059dd
> > ("net: fix pskb_trim_rcsum_slow() with odd trim offset") and
> > commit d48051c5b837 ("net/mlx5e: fix csum adjustments caused by RXFCS").
> >
> > Fixes: ca4ef4574f1e ("ip: fix IP_CHECKSUM handling")
>
> I do not believe you fix any bug here...
>
> I do not know of any inet protocol having odd header sizes.

That offset is payload offset, but yeah, the payload offset must be
aligned some way. Good point!

Let's drop it.

^ permalink raw reply

* Re: [PATCH 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
From: Russell King - ARM Linux @ 2018-11-08 22:21 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, Kishon Vijay Abraham I, Alexandre Belloni,
	Quentin Schulz, Manu Gautam, Tony Lindgren, netdev,
	Antoine Tenart, Sekhar Nori, linux-kernel, Maxime Ripard,
	Chen-Yu Tsai, Chunfeng Yun, linux-mediatek, Vivek Gautam,
	Carlo Caione, linux-amlogic, linux-arm-kernel, Matthias Brugger
In-Reply-To: <20181108003617.10334-5-grygorii.strashko@ti.com>

On Wed, Nov 07, 2018 at 06:36:16PM -0600, Grygorii Strashko wrote:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7a37a37..fb28b71 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1165,28 +1165,17 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
>   */
>  static int mvpp22_comphy_init(struct mvpp2_port *port)
>  {
> -	enum phy_mode mode;
> +	int submode;
>  	int ret;
>  
>  	if (!port->comphy)
>  		return 0;
>  
> -	switch (port->phy_interface) {
> -	case PHY_INTERFACE_MODE_SGMII:
> -	case PHY_INTERFACE_MODE_1000BASEX:
> -		mode = PHY_MODE_SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_2500BASEX:
> -		mode = PHY_MODE_2500SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_10GKR:
> -		mode = PHY_MODE_10GKR;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> +	submode = port->phy_interface;
> +	if (submode == PHY_INTERFACE_MODE_1000BASEX)
> +		submode = PHY_INTERFACE_MODE_SGMII;

If the intention is to move the ethernet PHY mode into the generic PHY
layer, I'd suggest that the equivalence of 1000base-X and SGMII is
handled in the generic PHY driver rather than ethernet drivers.  Just
move this into this hunk of the comphy driver:

> @@ -517,10 +519,14 @@ static int mvebu_comphy_set_mode(struct phy *phy,
>  {
>  	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>  
> -	if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
>  		return -EINVAL;
>  
>  	lane->mode = mode;
> +	lane->submode = submode;
>  	return 0;
>  }

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [Patch net] ip: fix csum_sub() with csum_block_sub()
From: Eric Dumazet @ 2018-11-08 22:16 UTC (permalink / raw)
  To: Cong Wang, Dimitris Michailidis; +Cc: netdev, Paolo Abeni, Willem de Bruijn
In-Reply-To: <20181108220407.17691-1-xiyou.wangcong@gmail.com>

On Thu, Nov 8, 2018 at 2:04 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> When subtracting the checksum of a block of data,
> csum_block_sub() must be used to respect the offset.
>
> We learned this lesson from both commit d55bef5059dd
> ("net: fix pskb_trim_rcsum_slow() with odd trim offset") and
> commit d48051c5b837 ("net/mlx5e: fix csum adjustments caused by RXFCS").
>
> Fixes: ca4ef4574f1e ("ip: fix IP_CHECKSUM handling")

I do not believe you fix any bug here...

I do not know of any inet protocol having odd header sizes.

^ permalink raw reply

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Marcel Holtmann @ 2018-11-09  7:49 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Kai-Heng Feng, Luca Coelho, Kalle Valo, Emmanuel Grumbach,
	Johannes Berg, David S. Miller, Intel Linux Wireless,
	open list:NFC SUBSYSTEM, netdev, linux-kernel, linux,
	João Paulo Rechi Vita
In-Reply-To: <20181109000800.15431-1-jprvita@gmail.com>

Hi Joao Paulo,

>>>> I think Canonical were facing some wifi fw load error from some 8260
>>>> earlier module during the BT still loading the fw.
>>>> I believe we had later 8260 sku that fixed this issue.
>>> 
>>> But there are already 8260 that is affected by this bug in the wild.
>>> 
>>> Search "Bluetooth: hci0: Failed to send firmware data (-38)” and there are lots of user are affected.
>> 
>> which SKUs are these actually. What are the initial details about the boot loader. For the Bluetooth side, you should be able to grab them from dmesg or by running btmon.
>> 
>> So I am not in favor of this kind of hack and creating dependencies between drivers. If you only have a hammer, then everything looks like a nail. And this is a massive hammer trying to squash everything. This problem needs to be debugged. And this starts by providing affected SKU information and firmware information. So get the details about the SKU and its Bluetooth and WiFi boot loaders.
>> 
> 
> I have a Lenovo Yoga 900 which presents this problem and has the same bootloader / firmware information as Kai-Heng already posted:
> 
> [    5.992426] Bluetooth: Core ver 2.22
> [    5.992438] Bluetooth: HCI device and connection manager initialized
> [    5.992442] Bluetooth: HCI socket layer initialized
> [    5.992444] Bluetooth: L2CAP socket layer initialized
> [    5.992450] Bluetooth: SCO socket layer initialized
> [    6.004941] Bluetooth: hci0: Bootloader revision 0.0 build 2 week 52 2014
> [    6.010922] Bluetooth: hci0: Device revision is 5
> [    6.010923] Bluetooth: hci0: Secure boot is enabled
> [    6.010924] Bluetooth: hci0: OTP lock is enabled
> [    6.010925] Bluetooth: hci0: API lock is enabled
> [    6.010926] Bluetooth: hci0: Debug lock is disabled
> [    6.010927] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> [    6.014253] bluetooth hci0: firmware: direct-loading firmware intel/ibt-11-5.sfi
> [    6.014256] Bluetooth: hci0: Found device firmware: intel/ibt-11-5.sfi
> [    6.613961] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> [    6.613966] Bluetooth: BNEP filters: protocol multicast
> [    6.613974] Bluetooth: BNEP socket layer initialized
> [    6.983804] Bluetooth: hci0: Failed to send firmware data (-38)
> 
> And the following product id and revision, from usb-devices:
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=06 Cnt=02 Dev#=  3 Spd=12  MxCh= 0
> D:  Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=8087 ProdID=0a2b Rev=00.01
> C:  #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I:  If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> 
> I understand the drawbacks with the approach presented here and lack of clear explanation of the problem, but I can confirm these patches work around the problem on my machine. Is there any extra info or test result I can provide to help debug this? I can also dedicate time to help write a different solution if some guidance is provided.
> 
> Kai-Heng, did you end up filling a Bugzilla entry for this?
> 
> Please CC-me on the replies as I'm not receiving emails from linux-bluetooth or linux-wireless anymore.

our hardware teams from the Bluetooth and WiFi side really need to look at this. An inter-dependency between the firmware loading of two otherwise independent drivers is really not what I want to see upstream. However I have no idea which boot loaders are affected or if this is something that might be even possible to be fixed in operational firmware. If you can create a binary btmon trace file with the error happening and getting really every single message from the boot we might get a bit further to understand where it goes wrong.

Regards

Marcel

^ 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