* [PATCH 0/1] NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying ncm bind common code
From: Daniele Palmas @ 2016-11-23 12:36 UTC (permalink / raw)
To: Bjørn Mork, Oliver Neukum; +Cc: linux-usb, netdev, Daniele Palmas
Some latest QC based modems seem not to properly accept altsetting
toggling in cdc_ncm_bind_common, making them to fail. The workaround
was to introduce an empirically decided pause to avoid the failure.
This patch introduces a different approach: for MBIM devices, instead
of toggling interfaces, the MBIM class-specific request code
RESET_FUNCTION is used in order to reset the function to its initial
state, removing the need for the pause.
Patch has been tested with a few Telit QC and Intel based MBIM modems.
Patch has also been tested with an Intel NCM based device, for
regression checking.
Daniele Palmas (1):
NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying
ncm bind common code
drivers/net/usb/cdc_ncm.c | 39 +++++++++++++++++++++++++++------------
include/uapi/linux/usb/cdc.h | 1 +
2 files changed, 28 insertions(+), 12 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: verify root pointer before dereferncing it
From: Daniel Borkmann @ 2016-11-23 11:29 UTC (permalink / raw)
To: Cong Wang, John Fastabend
Cc: Jiri Pirko, Roi Dayan, David S. Miller,
Linux Kernel Network Developers, Jiri Pirko, Or Gerlitz,
Cong Wang
In-Reply-To: <CAM_iQpWhGfrZgBD1Li+efZCv4Q6uGno1ttXcBL4C_F+4es4e_A@mail.gmail.com>
On 11/23/2016 06:24 AM, Cong Wang wrote:
> On Tue, Nov 22, 2016 at 3:36 PM, John Fastabend
> <john.fastabend@gmail.com> wrote:
>> On 16-11-22 12:41 PM, Daniel Borkmann wrote:
>>> On 11/22/2016 08:28 PM, Cong Wang wrote:
>>>> On Tue, Nov 22, 2016 at 8:11 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Tue, Nov 22, 2016 at 05:04:11PM CET, daniel@iogearbox.net wrote:
>>>>>> Hmm, I don't think we want to have such an additional test in fast
>>>>>> path for each and every classifier. Can we think of ways to avoid that?
>>>>>>
>>>>>> My question is, since we unlink individual instances from such
>>>>>> tp-internal
>>>>>> lists through RCU and release the instance through call_rcu() as
>>>>>> well as
>>>>>> the head (tp->root) via kfree_rcu() eventually, against what are we
>>>>>> protecting
>>>>>> setting RCU_INIT_POINTER(tp->root, NULL) in ->destroy() callback?
>>>>>> Something
>>>>>> not respecting grace period?
>>>>>
>>>>> If you call tp->ops->destroy in call_rcu, you don't have to set tp->root
>>>>> to null.
>>>
>>> But that's not really an answer to my question. ;)
>>>
>>>> We do need to respect the grace period if we touch the globally visible
>>>> data structure tp in tcf_destroy(). Therefore Roi's patch is not
>>>> fixing the
>>>> right place.
>>>
>>> I think there may be multiple issues actually.
>>>
>>> At the time we go into tc_classify(), from ingress as well as egress side,
>>> we're under RCU, but BH variant. In cls delete()/destroy() callbacks, we
>>> everywhere use call_rcu() and kfree_rcu(), same as for tcf_destroy() where
>>> we use kfree_rcu() on tp, although we iterate tps (and implicitly inner
>>> filters)
>>> via rcu_dereference_bh() from reader side. Is there a reason why we don't
>>> use call_rcu_bh() variant on destruction for all this instead?
>>
>> I can't think of any if its all under _bh we can convert the call_rcu to
>> call_rcu_bh it just needs an audit.
>>
>>> Just looking at cls_bpf and others, what protects
>>> RCU_INIT_POINTER(tp->root,
>>> NULL) against? The tp is unlinked in tc_ctl_tfilter() from the tp chain in
>>> tcf_destroy() cases. Still active readers under RCU BH can race against
>>> this
>>> (tp->root being NULL), as the commit identified. Only the get() callback
>>> checks
>>> for head against NULL, but both are serialized under rtnl, and the only
>>> place
>>> we call this is tc_ctl_tfilter(). Even if we create a new tp, head
>>> should not
>>> be NULL there, if it was assigned during the init() cb, but contains an
>>> empty
>>> list. (It's different for things like cls_cgroup, though.) So, I'm
>>> wondering
>>> if the RCU_INIT_POINTER(tp->root, NULL) can just be removed instead
>>> (unless I'm
>>> missing something obvious)?
>>
>> Just took a look at this I think there are a couple possible solutions.
>> The easiest is likely to fix all the call sites so that 'tp' is unlinked
>> before calling the destroy() handlers AND not doing the NULL set. I only
>> see one such call site where destroy is called before unlinking at the
>> moment. This should enforce that after a grace period there is no path
>> to reach the classifiers because 'tp' is unlinked. Calling destroy
>> before unlinking 'tp' however could cause a small race between grace
>> period of 'tp' and grace period of the filter.
>>
>> Another would be to only call the destroy path from the call_rcu path
>> of the 'tp' object so that destroy is only ever called after the object
>> is guaranteed to be unlinked from the tc_filter path.
>>
>> I think both solutions would be fine.
>>
>> Cong were you working on one of these? Or do you have another idea?
>
> Yeah, this is basic what I think as well, however, both are hard.
> On one hand, we can't detach the tp from the global singly-linked list
> before tcf_destroy() since we rely on its return value to make this decision.
> On the other hand, it is a singly-linked list, we have to pass in the address
> of its previous pointer to rcu callback to remove it, it seems racy as well
> since we modify a previous pointer which is still visible globally...
Can't we drop the 'force' parameter from tcf_destroy() and related cls
destroy() callbacks, and change the logic roughly like this:
[...]
case RTM_DELTFILTER:
err = tp->ops->delete(tp, fh, &drop_tp);
if (err == 0) {
struct tcf_proto *next = rtnl_dereference(tp->next);
tfilter_notify(net, skb, n, tp,
t->tcm_handle,
RTM_DELTFILTER, false);
if (drop_tp) {
RCU_INIT_POINTER(*back, next);
tcf_destroy(tp);
}
}
goto errout;
[...]
This one was the only tcf_destroy() instance with force=false. Why can't
the prior delete() callback make the decision whether the tp now has no
further internal filters and thus can be dropped. Afaik, delete() and
destroy() are protected by RTNL anyway. Thus, we could unlink the tp from
the list before tcf_destroy(), which should then work with grace period
as well. Given we remove the setting of tp->root to NULL, any outstanding
readers for that grace period should either still execute the 'scheduled
for removal' filter we just dropped, or find an empty list of filters.
> Hmm, perhaps we really have to switch to a doubly-linked list, that is
> list_head. I need to double check. And also the semantic of ->destroy()
> needs to revise too.
Can you elaborate why double-linked list? Isn't the tp list always protected
from modifications via RTNL in control path, and walked via rcu_dereference_bh()
in data path?
> So yeah, my commit should be blamed. :-/
^ permalink raw reply
* Re: net/arp: ARP cache aging failed.
From: Eric Dumazet @ 2016-11-23 12:05 UTC (permalink / raw)
To: Julian Anastasov; +Cc: yuehaibing, davem, netdev
In-Reply-To: <alpine.LFD.2.11.1611230928140.1659@ja.home.ssi.bg>
On Wed, 2016-11-23 at 10:33 +0200, Julian Anastasov wrote:
> Hello,
>
> On Wed, 23 Nov 2016, yuehaibing wrote:
>
> > As to my topo,HOST1 and HOST3 share one route on HOST2, tcp connection between HOST2 and HOST3 may call tcp_ack to set dst->pending_confirm.
> >
> > So dst_neigh_output may wrongly freshed n->confirmed which stands for HOST1,however HOST1'MAC had been changed.
> >
> > The possibility of this occurred Significantly increases ,when ping and TCP transaction are set the same processor affinity on the HOST2.
> >
> > It seems that the issue is brought in commit 5110effee8fde2edfacac9cd12a9960ab2dc39ea ("net: Do delayed neigh confirmation.").
>
> Bad news. Problem is not in delayed confirmation but
> in the mechanism to use same dst for different neighbours on
> LAN. We don't have a dst->neighbour reference anymore.
>
> For IPv4 this is related to rt->rt_uses_gateway but
> also to DST_NOCACHE. In the other cases we can not call
> dst_confirm, may be we should lookup the neigh entry instead.
> But we need a way to reduce such lookups on every packet,
> for example, by remembering in struct sock and checking if
> some bits of jiffies (at least 4-5) are changed from
> previous lookup.
I thought bonding would keep the MAC address 'alive'.
If TCP packets are confirmed, this means the old MAC address is still
valid, what am I missing here ?
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: egress all frames
From: Stefan Eichenberger @ 2016-11-23 12:00 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Stefan Eichenberger, vivien.didelot, f.fainelli, netdev
In-Reply-To: <20161122190206.GE14947@lunn.ch>
On Tue, Nov 22, 2016 at 08:02:06PM +0100, Andrew Lunn wrote:
> On Tue, Nov 22, 2016 at 07:37:33PM +0100, Stefan Eichenberger wrote:
> > Hi Andrew
> >
> > On Tue, Nov 22, 2016 at 04:03:30PM +0100, Andrew Lunn wrote:
> > > On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> > > > Egress multicast and egress unicast is only enabled for CPU/DSA ports
> > > > but for switching operation it seems it should be enabled for all ports.
> > > > Do I miss something here?
> > > >
> > > > I did the following test:
> > > > brctl addbr br0
> > > > brctl addif br0 lan0
> > > > brctl addif br0 lan1
> > > >
> > > > In this scenario the unicast and multicast packets were not forwarded,
> > > > therefore ARP requests were not resolved, and no connection could be
> > > > established.
> > >
> > > Hi Stefan
> > >
> > > This is probably specific to the 6097 family. It works fine without
> > > this on other devices. Creating a bridge like above and pinging across
> > > it is one of my standard tests. But i only test modern devices like
> > > the 6165, 6352, 6351, 6390 families.
> >
> > Okay perfect, I wasn't 100% sure if I would have to configure something
> > additionally.
>
> No. The idea is you treat the interfaces as normal interfaces. You
> should not need to do anything additional to what you would do with a
> normal interface, when adding it to a bridge.
>
> > > In fact, you might need to review all the code and look where
> > > mv88e6xxx_6095_family(chip) is used and consider if you need to add
> > > mv88e6xxx_6097_family(chip). e.g.
> > >
> > > if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
> > > /* Set the upstream port this port should use */
> > > reg |= dsa_upstream_port(ds);
> > > /* enable forwarding of unknown multicast addresses to
> > > * the upstream port
> > > */
> > > if (port == dsa_upstream_port(ds))
> > > reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
> > > }
> > >
> > > Maybe this is your problem?
> >
> > I think I still don't understand exactly how the driver works.
> >
> > My problem is that the multicast and broadcast frames are filtered and
> > the following counter is increasing in ethtool:
> > sw_in_filtered: 596
>
> This is not what is supposed to happen. Broadcast and multicast frames
> should go to all ports in the bridge. There are two different ways
> this can happen:
>
> 1) The mv88e6xxx driver started out with the host doing all bridge
> operations. The switch forwards all frames to the software bridge, and
> the software bridge then sends them out another port if needed.
>
> 2) We later added support for hardware bridging. That is, the switch
> itself bridges frames between ports. It will only pass frames to the
> software bridge if it does not know what to do with a frame itself.
Thanks for this explanation it helped a lot.
>
> Now, the different families are not 100% compatible with each
> other. We never had access to a 6097, so it has not been tested
> recently, and we have probably broken it... My guess would be,
> anywhere mv88e6xxx_6095_family(chip) is used, there also needs to be
> an mv88e6xxx_6097_family(chip). But i could be wrong.
I think I probably found the problem. For EDSA type switches the bit
PORT_CONTROL_FORWARD_UNKNOWN_MC is set on the cpu port but not for DSA
type switches. Broadcast addresses are threaded as multicast addresses,
so unknown frames will never leave the switch.
Do you know if there is a reason why this bit isn't set for DSA type
switches too? The patch would be extremely simple and it seems to work
perfectly with this bit set on the CPU port.
Thanks
Stefan
^ permalink raw reply
* Re: [PATCH net-next 4/5] net: phy: bcm7xxx: Add support for downshift/Wirespeed
From: Allan W. Nielsen @ 2016-11-23 11:45 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, netdev, davem, bcm-kernel-feedback-list,
raju.lakkaraju, vivien.didelot
In-Reply-To: <1902f0f0-46e5-d3b3-90c1-10867f4fb826@gmail.com>
Hi,
On 22/11/16 12:07, Florian Fainelli wrote:
> On 11/22/2016 12:02 PM, Andrew Lunn wrote:
> >> +static int bcm7xxx_28nm_set_tunable(struct phy_device *phydev,
> >> + struct ethtool_tunable *tuna,
> >> + const void *data)
> >> +{
> >> + u8 count = *(u8 *)data;
> >> + int ret;
> >> +
> >> + switch (tuna->id) {
> >> + case ETHTOOL_PHY_DOWNSHIFT:
> >> + ret = bcm_phy_downshift_set(phydev, count);
> >> + break;
> >> + default:
> >> + return -EOPNOTSUPP;
> >> + }
> >> +
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Disable EEE advertisment since this prevents the PHY
> >> + * from successfully linking up, trigger auto-negotiation restart
> >> + * to let the MAC decide what to do.
> >> + */
> >> + ret = bcm_phy_set_eee(phydev, count == DOWNSHIFT_DEV_DISABLE);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return genphy_restart_aneg(phydev);
> >> +}
> >
> > Hi Florian
> >
> > Is the locking O.K. here? The core code does not take the phy lock.
> > But i think your shadow register accesses at least need to be
> > protected by the lock?
>
> There should be some kind of protection, but I was expecting it to be
> done at the caller level, so that when {get,set}_tunable run, they are
> serialized with respect to each other, clearly, by looking at the code,
> this is not the case.
>
> >
> > Maybe we should think about this locking a bit. It is normal for the
> > lock to be held when using ops in the phy driver structure. The
> > exception is suspend/resume. Maybe we should also take the lock before
> > calling the phydev->drv->get_tunable() and phydev->drv->set_tunable()?
>
> Yes, that certainly seems like a good approach to me, let me cook a
> patch doing that.
Just for my understanding (such that I will not make the same mistake again)...
Why is it that phy functions such as get_wol needs to take the phy_lock and
others like get_tunable does not.
I do understand the arguments on why the lock should be held by the caller of
get_tunable, but I do not understand why the same argument does not apply for
get_wol.
/Allan
^ permalink raw reply
* Re: Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-23 11:43 UTC (permalink / raw)
To: Lars Persson, Joao Pinto
Cc: Giuseppe CAVALLARO, Rayagond Kokatanur, Rabin Vincent, mued dib,
David Miller, Jeff Kirsher, jiri@mellanox.com,
saeedm@mellanox.com, idosch@mellanox.com, netdev,
linux-kernel@vger.kernel.org, CARLOS.PALMINHA@synopsys.com,
Andreas Irestål, alexandre.torgue@st.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <899DC02E-84BB-489E-A1FE-5D8F3BB795B6@axis.com>
On 23-11-2016 11:41, Lars Persson wrote:
>
>> 23 nov. 2016 kl. 12:11 skrev Joao Pinto <Joao.Pinto@synopsys.com>:
>>
>> Hi Peppe and Lars,
>>
>>> On 23-11-2016 10:59, Giuseppe CAVALLARO wrote:
>>> Hello Joao, Lars.
>>>
>>>> On 11/22/2016 3:16 PM, Joao Pinto wrote:
>>>>>> Ok, it makes sense.
>>>>>> Just for curiosity the target setup is the following:
>>>>>> https://www.youtube.com/watch?v=8V-LB5y2Cos
>>>>>> but instead of using internal drivers, we desire to use mainline drivers only.
>>>>>>
>>>>>> Thanks!
>>>> Regarding this subject, I am thinking of making the following adaption:
>>>>
>>>> a) delete ethernet/synopsys
>>>> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>>>>
>>>> and send you a patch for you to evaluate. Both agree with the approach?
>>>> To have a new work base would be important, because I will add to the "new"
>>>> structure some missing QoS features like Multichannel support, CBS and later TSN.
>>>
>>> IMO, we have to agree on a common strategy making the change for
>>> net-next; I imaged the following steps:
>>
>> Yes it makes totally sense.
>>
>>>
>>> - to port missing feature or fixes from ethernet/synopsys
>>> inside the stmmac taking care about the documentation too.
>>
>> @Lars: You are familiar with the synopsys qos driver. Could you please do this
>> porting. You can also make an analysis of what to port and I can do the porting
>> for you if you don't have the availability for it.
>
> As my main duty is changing diapers until March next year, please go ahead with this step if you can spend time on it before I am back in office.
Congratulations :)!
>
> Rabin Vincent can review and test that the port works properly on our Artpec-chips that use dwc_eth_qos.c today.
>
> The main porting step is to implement the device tree binding in bindings/net/snps,dwc-qos-ethernet.txt. Also our chip has a strict requirement that the phy is enabled when the SWR reset bit is set (it needs a tx clock to complete the reset).
>
> - Lars
Ok, I will do the task.
@Peppe: Agree with the plan?
>
>>
>>> - remove ethernet/synopsys
>>> - rename ethernet/stmicro/stmmac to ethernet/synopsys
>>
>> I volunteer to do this task.
>>
>>>
>>> These latest two have some relevant impacts.
>>>
>>> This change should be propagated to all the platforms that are using:
>>> CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
>>> plus device-tree compatibility.
>>
>> I volunteer to do this task also.
>>
>>>
>>> - enhance the stmmac with new features and new glue (part of these
>>> can be anticipated for sure).
>>
>> I have to implement 3 new features for now, but I will take some time for it, so
>> I would suggest to make the previous task and incrementally add features.
>>
>>>
>>> what do you think? does it make sense? If yes, we can also
>>> understand how/who starts.
>>>
>>> Regards,
>>> Peppe
>>
>> Thanks and regards.
>>
>> Joao
>>
>>>
>>>> Thanks.
>>>
>>
^ permalink raw reply
* Re: Synopsys Ethernet QoS Driver
From: Lars Persson @ 2016-11-23 11:41 UTC (permalink / raw)
To: Joao Pinto
Cc: Giuseppe CAVALLARO, Rayagond Kokatanur, Rabin Vincent, mued dib,
David Miller, Jeff Kirsher, jiri@mellanox.com,
saeedm@mellanox.com, idosch@mellanox.com, netdev,
linux-kernel@vger.kernel.org, CARLOS.PALMINHA@synopsys.com,
Andreas Irestål, alexandre.torgue@st.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <7c259adb-5c73-f997-6b96-5be427157b08@synopsys.com>
> 23 nov. 2016 kl. 12:11 skrev Joao Pinto <Joao.Pinto@synopsys.com>:
>
> Hi Peppe and Lars,
>
>> On 23-11-2016 10:59, Giuseppe CAVALLARO wrote:
>> Hello Joao, Lars.
>>
>>> On 11/22/2016 3:16 PM, Joao Pinto wrote:
>>>>> Ok, it makes sense.
>>>>> Just for curiosity the target setup is the following:
>>>>> https://www.youtube.com/watch?v=8V-LB5y2Cos
>>>>> but instead of using internal drivers, we desire to use mainline drivers only.
>>>>>
>>>>> Thanks!
>>> Regarding this subject, I am thinking of making the following adaption:
>>>
>>> a) delete ethernet/synopsys
>>> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>>>
>>> and send you a patch for you to evaluate. Both agree with the approach?
>>> To have a new work base would be important, because I will add to the "new"
>>> structure some missing QoS features like Multichannel support, CBS and later TSN.
>>
>> IMO, we have to agree on a common strategy making the change for
>> net-next; I imaged the following steps:
>
> Yes it makes totally sense.
>
>>
>> - to port missing feature or fixes from ethernet/synopsys
>> inside the stmmac taking care about the documentation too.
>
> @Lars: You are familiar with the synopsys qos driver. Could you please do this
> porting. You can also make an analysis of what to port and I can do the porting
> for you if you don't have the availability for it.
As my main duty is changing diapers until March next year, please go ahead with this step if you can spend time on it before I am back in office.
Rabin Vincent can review and test that the port works properly on our Artpec-chips that use dwc_eth_qos.c today.
The main porting step is to implement the device tree binding in bindings/net/snps,dwc-qos-ethernet.txt. Also our chip has a strict requirement that the phy is enabled when the SWR reset bit is set (it needs a tx clock to complete the reset).
- Lars
>
>> - remove ethernet/synopsys
>> - rename ethernet/stmicro/stmmac to ethernet/synopsys
>
> I volunteer to do this task.
>
>>
>> These latest two have some relevant impacts.
>>
>> This change should be propagated to all the platforms that are using:
>> CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
>> plus device-tree compatibility.
>
> I volunteer to do this task also.
>
>>
>> - enhance the stmmac with new features and new glue (part of these
>> can be anticipated for sure).
>
> I have to implement 3 new features for now, but I will take some time for it, so
> I would suggest to make the previous task and incrementally add features.
>
>>
>> what do you think? does it make sense? If yes, we can also
>> understand how/who starts.
>>
>> Regards,
>> Peppe
>
> Thanks and regards.
>
> Joao
>
>>
>>> Thanks.
>>
>
^ permalink raw reply
* Re: [PATCH] ipv6:ipv6_pinfo dereferenced after NULL check
From: Hannes Frederic Sowa @ 2016-11-23 11:32 UTC (permalink / raw)
To: r.thapliyal, Manjeet Pawar, davem@davemloft.net,
kuznet@ms2.inr.ac.ru, jmorris@namei.org, yoshfuji@linux-ipv6.org,
kaber@trash.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: PANKAJ MISHRA, Ajeet Kumar Yadav
In-Reply-To: <20161123044535epcms5p37a2a3f2c2c071fdac1ddb6b1a6c02cf6@epcms5p3>
On 23.11.2016 05:45, Rohit Thapliyal wrote:
> |>On 22.11.2016 07:27, Manjeet Pawar wrote:
> >> From: Rohit Thapliyal <r.thapliyal@samsung.com <mailto:r.thapliyal@samsung.com>>
> >>
> >> np checked for NULL and then dereferenced. It should be modified
> >> for NULL case.
> >>
> >> Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com
> <mailto:r.thapliyal@samsung.com>>>
> >> Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com
> <mailto:manjeet.p@samsung.com>>>
> >> ---
> >> net/ipv6/ip6_output.c | 9 +++++----
> >> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> >> index 1dfc402..c2afa14 100644
> >> --- a/net/ipv6/ip6_output.c
> >> +++ b/net/ipv6/ip6_output.c
> >> @@ -205,14 +205,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff
> *skb, struct flowi6 *fl6,
> >> /*
> >> * Fill in the IPv6 header
> >> */
> >> - if (np)
> >> + if (np) {
> >> hlimit = np->hop_limit;
> >> + ip6_flow_hdr(
> >> + hdr, tclass, ip6_make_flowlabel(
> >> + net, skb, fl6->flowlabel,
> >> + np->autoflowlabel, fl6));
> >> + }
> >> if (hlimit < 0)
> >> hlimit = ip6_dst_hoplimit(dst);
> >>
> >> - ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
> >> - np->autoflowlabel, fl6));
> >> -
> >> hdr->payload_len = htons(seg_len);
> >> hdr->nexthdr = proto;
> >> hdr->hop_limit = hlimit;
> >>
> >
> >
> >We always should initialize hdr and not skip the ip6_flow_hdr call.|
>
> |if np becomes NULL, then anyways hdr won't be initialized due to NULL pointer
> dereference ip6_make_flowlabel.|
Which we would see as a crash. So far no crash has been reported in this
code. Doing a quick code review on the paths leading to ip6_xmit,
inet6_sk must always be set to actually reach up to this point.
Otherwise we would have crashes on all code paths much earler.
Anyway, I would be fine to keep the NULL check in this path, it looks
better because of the inet6_sk you pointed out above but I would
recommend to just use a "np ? np->autoflowlabel :
ip6_default_np_autolabel(net) in the ip6_flow_hdr function.
> |>Do you saw a bug or did you find this by code review? I wonder if np can
> >actually be NULL at this point. Maybe we can just eliminate the NULL check.|
>
> |
>
>
> I must admit that I found it just by code review, and so far didn't face any
> crash whatsoever.
> As we can see in inet6_sk, np could be NULL. Thus, the NULL check seems justified.
Thanks for looking at this!
Bye,
Hannes
^ permalink raw reply
* Re: Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-23 11:10 UTC (permalink / raw)
To: Giuseppe CAVALLARO, Joao Pinto, Lars Persson
Cc: Rayagond Kokatanur, Rabin Vincent, mued dib, David Miller,
Jeff Kirsher, jiri@mellanox.com, saeedm@mellanox.com,
idosch@mellanox.com, netdev, linux-kernel@vger.kernel.org,
CARLOS.PALMINHA@synopsys.com, Andreas Irestål,
alexandre.torgue@st.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <c37c47e1-8e21-1b11-ed15-6b899ed1dd03@st.com>
Hi Peppe and Lars,
On 23-11-2016 10:59, Giuseppe CAVALLARO wrote:
> Hello Joao, Lars.
>
> On 11/22/2016 3:16 PM, Joao Pinto wrote:
>>> Ok, it makes sense.
>>> > Just for curiosity the target setup is the following:
>>> > https://www.youtube.com/watch?v=8V-LB5y2Cos
>>> > but instead of using internal drivers, we desire to use mainline drivers only.
>>> >
>>> > Thanks!
>> Regarding this subject, I am thinking of making the following adaption:
>>
>> a) delete ethernet/synopsys
>> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>>
>> and send you a patch for you to evaluate. Both agree with the approach?
>> To have a new work base would be important, because I will add to the "new"
>> structure some missing QoS features like Multichannel support, CBS and later TSN.
>
> IMO, we have to agree on a common strategy making the change for
> net-next; I imaged the following steps:
Yes it makes totally sense.
>
> - to port missing feature or fixes from ethernet/synopsys
> inside the stmmac taking care about the documentation too.
@Lars: You are familiar with the synopsys qos driver. Could you please do this
porting. You can also make an analysis of what to port and I can do the porting
for you if you don't have the availability for it.
> - remove ethernet/synopsys
> - rename ethernet/stmicro/stmmac to ethernet/synopsys
I volunteer to do this task.
>
> These latest two have some relevant impacts.
>
> This change should be propagated to all the platforms that are using:
> CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
> plus device-tree compatibility.
I volunteer to do this task also.
>
> - enhance the stmmac with new features and new glue (part of these
> can be anticipated for sure).
I have to implement 3 new features for now, but I will take some time for it, so
I would suggest to make the previous task and incrementally add features.
>
> what do you think? does it make sense? If yes, we can also
> understand how/who starts.
>
> Regards,
> Peppe
Thanks and regards.
Joao
>
>> Thanks.
>
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Jisheng Zhang @ 2016-11-23 11:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Thomas Petazzoni, Andrew Lunn, Jason Cooper,
netdev, linux-kernel, Gregory CLEMENT, Marcin Wojtas,
David S. Miller, Sebastian Hesselbarth
In-Reply-To: <9432400.S1OrxC027t@wuerfel>
Hi Arnd,
On Wed, 23 Nov 2016 11:15:32 +0100 Arnd Bergmann wrote:
> On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote:
> > On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote:
> >
> > > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote:
> > > > +#ifdef CONFIG_64BIT
> > > > + void *data_tmp;
> > > > +
> > > > + /* In Neta HW only 32 bits data is supported, so in order to
> > > > + * obtain whole 64 bits address from RX descriptor, we store
> > > > + * the upper 32 bits when allocating buffer, and put it back
> > > > + * when using buffer cookie for accessing packet in memory.
> > > > + * Frags should be allocated from single 'memory' region,
> > > > + * hence common upper address half should be sufficient.
> > > > + */
> > > > + data_tmp = mvneta_frag_alloc(pp->frag_size);
> > > > + if (data_tmp) {
> > > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32;
> > > > + mvneta_frag_free(pp->frag_size, data_tmp);
> > > > + }
> > > >
> > >
> > > How does this work when the region spans a n*4GB address boundary?
> >
> > indeed. We also make use of this driver on 64bit platforms. We use
> > different solution to make the driver 64bit safe.
> >
> > solA: make use of the reserved field in the mvneta_rx_desc, such
> > as reserved2 etc. Yes, the field is marked as "for future use, PnC", but
> > now it's not used at all. This is one possible solution however.
>
> Right, this sounds like the most straightforward choice.
>
> > solB: allocate a shadow buf cookie during init, e.g
> >
> > rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL);
> >
> > then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in
> > the shadow buf cookie, e.g
> > static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
> > u32 phys_addr, u32 cookie,
sorry, this line should be:
u32 phys_addr, void *cookie
> > struct mvneta_rx_queue *rxq)
> >
> > {
> > int i;
> >
> > rx_desc->buf_cookie = cookie;
> > rx_desc->buf_phys_addr = phys_addr;
> > i = rx_desc - rxq->descs;
> > rxq->descs_bufcookie[i] = cookie;
> > }
> >
> > then fetch the desc from the shadow buf cookie in all code path, such
> > as mvneta_rx() etc.
> >
> > Both solutions should not have the problems pointed out by Arnd.
>
> Wait, since you compute an index 'i' here, can't you just store 'i'
> directly in the descriptor instead of the pointer?
>
we need to store the pointer, it's to store the buffer allocated by
mvneta_frag_alloc()
Thanks,
Jisheng
^ permalink raw reply
* [PATCH] cxgb4: fix memory leak on txq_info
From: Colin King @ 2016-11-23 11:02 UTC (permalink / raw)
To: Hariprasad S, netdev; +Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently if txq_info->uldtxq cannot be allocated then
txq_info->txq is being kfree'd (which is redundant because it
is NULL) instead of txq_info. Fix this by instead kfree'ing
txq_info.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
index 565a6c6..8098902 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
@@ -532,7 +532,7 @@ setup_sge_txq_uld(struct adapter *adap, unsigned int uld_type,
txq_info->uldtxq = kcalloc(txq_info->ntxq, sizeof(struct sge_uld_txq),
GFP_KERNEL);
if (!txq_info->uldtxq) {
- kfree(txq_info->uldtxq);
+ kfree(txq_info);
return -ENOMEM;
}
--
2.10.2
^ permalink raw reply related
* Re: Synopsys Ethernet QoS Driver
From: Giuseppe CAVALLARO @ 2016-11-23 10:59 UTC (permalink / raw)
To: Joao Pinto, Lars Persson
Cc: idosch@mellanox.com, alexandre.torgue@st.com, saeedm@mellanox.com,
netdev, linux-kernel@vger.kernel.org,
CARLOS.PALMINHA@synopsys.com, Rabin Vincent, mued dib,
jiri@mellanox.com, Rayagond Kokatanur, Jeff Kirsher,
Andreas Irestål, David Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <2eefdb8f-7e87-6009-6e50-c536d4b95dd6@synopsys.com>
Hello Joao, Lars.
On 11/22/2016 3:16 PM, Joao Pinto wrote:
>> Ok, it makes sense.
>> > Just for curiosity the target setup is the following:
>> > https://www.youtube.com/watch?v=8V-LB5y2Cos
>> > but instead of using internal drivers, we desire to use mainline drivers only.
>> >
>> > Thanks!
> Regarding this subject, I am thinking of making the following adaption:
>
> a) delete ethernet/synopsys
> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>
> and send you a patch for you to evaluate. Both agree with the approach?
> To have a new work base would be important, because I will add to the "new"
> structure some missing QoS features like Multichannel support, CBS and later TSN.
IMO, we have to agree on a common strategy making the change for
net-next; I imaged the following steps:
- to port missing feature or fixes from ethernet/synopsys
inside the stmmac taking care about the documentation too.
- remove ethernet/synopsys
- rename ethernet/stmicro/stmmac to ethernet/synopsys
These latest two have some relevant impacts.
This change should be propagated to all the platforms that are using:
CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
plus device-tree compatibility.
- enhance the stmmac with new features and new glue (part of these
can be anticipated for sure).
what do you think? does it make sense? If yes, we can also
understand how/who starts.
Regards,
Peppe
> Thanks.
^ permalink raw reply
* stmmac ethernet in kernel 4.4: coalescing related pauses?
From: Pavel Machek @ 2016-11-23 10:51 UTC (permalink / raw)
To: peppe.cavallaro, netdev, kernel list
[-- Attachment #1.1: Type: text/plain, Size: 933 bytes --]
Hi!
I'm debugging strange delays during transmit in stmmac driver. They
seem to be present in 4.4 kernel (and older kernels, too). Workload is
burst of udp packets being sent, pause, burst of udp packets, ...
Test code is attached, I use these parameters for testing:
./udp-test raw 10.0.0.6 1234 1000 100 30
The delays seem to be related to coalescing:
drivers/net/ethernet/stmicro/stmmac/common.h
#define STMMAC_COAL_TX_TIMER 40000
#define STMMAC_MAX_COAL_TX_TICK 100000
#define STMMAC_TX_MAX_FRAMES 256
If I lower the parameters, delays are gone, but I get netdev watchdog
backtrace followed by broken driver.
Any ideas what is going on there?
[I'm currently trying to get newer kernels working on affected
hardware.]
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: CMakeLists.txt --]
[-- Type: text/plain, Size: 589 bytes --]
cmake_minimum_required(VERSION 2.8.7)
project(streaming)
find_package(Boost REQUIRED COMPONENTS system)
set(SOURCES
udp-test.cpp)
add_executable(udp-test ${SOURCES})
if (BUILD_TESTS)
enable_testing()
endif()
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -std=c++11")
set_property(TARGET udp-test PROPERTY CXX_STANDARD 11)
target_link_libraries(udp-test boost_system pthread )
[-- Attachment #1.3: udp-test.cpp --]
[-- Type: text/x-c++src, Size: 6995 bytes --]
#include <boost/asio.hpp>
#include <boost/asio/steady_timer.hpp>
#include <iostream>
namespace asio = boost::asio;
class UdpSendTest
{
public:
UdpSendTest(asio::io_service& io_service, const std::string& dest_ip, int dest_port, int packet_size, int packet_count, int interval)
: io_service_(io_service),
timer_(io_service),
udp_socket_(io_service),
dest_ip_(std::move(dest_ip)),
dest_port_(dest_port),
packet_size_(packet_size),
packet_count_(packet_count),
period_(interval)
{
}
void start()
{
timer_.expires_from_now(period_);
timer_.async_wait(std::bind(&UdpSendTest::handleTimer, this, std::placeholders::_1));
try
{
udp_socket_.connect(asio::ip::udp::endpoint(boost::asio::ip::address::from_string(dest_ip_), dest_port_));
}
catch(boost::system::system_error e)
{
std::cerr<< "Could not connect:"<<e.what()<<std::endl;
}
}
private:
static_assert(std::chrono::steady_clock::is_steady, "steady_clock does not use the monotonic system clock. Please use a toolchain with full support for std::chrono!");
void sendPackets()
{
std::vector<unsigned char> buffer(packet_size_,0);
for (int i=0; i<packet_count_; i++)
{
if (buffer.size() > 1)
{
buffer[0] = i / 255;
buffer[1] = i % 255;
}
else
buffer[0]=i%255;
auto t0 = std::chrono::steady_clock::now();
try
{
udp_socket_.send(asio::buffer(buffer));
}
catch(boost::system::system_error& error)
{
std::cerr<<"Could not send UDP packet, reason: "<<error.what()<<std::endl;
}
auto delta_t = std::chrono::steady_clock::now() - t0;
auto delta_t_us = std::chrono::duration_cast<std::chrono::microseconds>(delta_t).count();
if (delta_t_us > 10000)
{
std::cout<<"Sending UDP packet took >10ms: "<<delta_t_us<<"us"<<std::endl;
}
if (delta_t_us > period_.count() * 1000)
{
std::cout<<"This would lead to a lost frame!"<<std::endl;
}
}
}
void handleTimer(boost::system::error_code ec)
{
if (ec)
{
std::cerr<<"Timer interrupted, exiting!"<<std::endl;
return;
}
sendPackets();
timer_.expires_at(timer_.expires_at() + period_);
timer_.async_wait(std::bind(&UdpSendTest::handleTimer, this, std::placeholders::_1));
}
asio::io_service& io_service_;
asio::steady_timer timer_;
asio::ip::udp::socket udp_socket_;
std::string dest_ip_;
int dest_port_;
int packet_size_;
int packet_count_;
std::chrono::milliseconds period_;
};
class UdpSendTestLowlevel
{
public:
UdpSendTestLowlevel(asio::io_service& io_service, const std::string& dest_ip, int dest_port, int packet_size, int packet_count, int interval)
: io_service_(io_service),
timer_(io_service),
dest_ip_(std::move(dest_ip)),
dest_port_(dest_port),
packet_size_(packet_size),
packet_count_(packet_count),
period_(interval)
{
}
void start()
{
timer_.expires_from_now(period_);
timer_.async_wait(std::bind(&UdpSendTestLowlevel::handleTimer, this, std::placeholders::_1));
socket_fd_ = socket(AF_INET, SOCK_DGRAM, 0);
if (socket_fd_ < 0)
std::cerr<<"could not create socket: " <<strerror(errno)<<std::endl;
auto h = gethostbyname(dest_ip_.c_str());
if (h == nullptr)
std::cerr<<"Could not find host: "<<dest_ip_<<std::endl;
server_addr_.sin_family = h->h_addrtype;
memcpy((char*)&server_addr_.sin_addr.s_addr, h->h_addr_list[0], h->h_length);
server_addr_.sin_port = htons(dest_port_);
client_addr_.sin_family = AF_INET;
client_addr_.sin_addr.s_addr = htonl(INADDR_ANY);
client_addr_.sin_port = htons(0);
auto rc = bind(socket_fd_,reinterpret_cast<sockaddr*>(&client_addr_), sizeof(client_addr_));
if (rc < 0)
std::cerr<<"Could not open Port"<<std::endl;
}
private:
static_assert(std::chrono::steady_clock::is_steady, "steady_clock does not use the monotonic system clock. Please use a toolchain with full support for std::chrono!");
void sendPackets()
{
std::vector<unsigned char> buffer(packet_size_,0);
for (int i=0; i<packet_count_; i++)
{
if (buffer.size() > 1)
{
buffer[0] = i / 255;
buffer[1] = i % 255;
}
else
buffer[0]=i%255;
auto t0 = std::chrono::steady_clock::now();
auto rc = sendto(socket_fd_, buffer.data(), buffer.size(), 0, (sockaddr* )&server_addr_, sizeof(server_addr_));
if (rc<0)
std::cerr<<"Could not send UDP packet"<<std::endl;
auto delta_t = std::chrono::steady_clock::now() - t0;
auto delta_t_us = std::chrono::duration_cast<std::chrono::microseconds>(delta_t).count();
if (delta_t_us > 10000)
{
std::cout<<"Sending UDP packet took >10ms: "<<delta_t_us<<"us"<<std::endl;
}
if (delta_t_us > period_.count() * 1000)
{
std::cout<<"This would lead to a lost frame!"<<std::endl;
}
}
}
void handleTimer(boost::system::error_code ec)
{
if (ec)
{
std::cerr<<"Timer interrupted, exiting!"<<std::endl;
return;
}
sendPackets();
timer_.expires_at(timer_.expires_at() + period_);
timer_.async_wait(std::bind(&UdpSendTestLowlevel::handleTimer, this, std::placeholders::_1));
}
asio::io_service& io_service_;
asio::steady_timer timer_;
std::string dest_ip_;
int dest_port_;
int packet_size_;
int packet_count_;
std::chrono::milliseconds period_;
int socket_fd_;
struct sockaddr_in client_addr_, server_addr_;
};
int main(int argc, char** argv)
{
if (argc < 7)
{
std::cout<<"usage: "<<argv[0]<<" [boost|raw] dest_ip dest_port packet_size packet_count interval_ms"<<std::endl;
return 1;
}
if (std::atoi(argv[4])<1)
{
std::cerr<<"Please select a packet size > 0 bytes!"<<std::endl;
return 1;
}
asio::io_service io_service;
std::string mode(argv[1]);
int dest_port = std::atoi(argv[3]);
int packet_size = std::atoi(argv[4]);
int packet_count = std::atoi(argv[5]);
int frame_interval = std::atoi(argv[6]);
int bytes_per_sec = packet_size * packet_count *(1000.f/frame_interval);
int bytes_per_sec_2 = (packet_size+12) * packet_count *(1000.f/frame_interval);
std::cout<<"Sending "<<packet_count<<" packets ("<<packet_size<<"b each) at an interval of "<<frame_interval<<"ms, expected data rate:"<<bytes_per_sec <<"b/s ("<<bytes_per_sec_2<<"b/s incl udp overhead)"<<std::endl;
if (bytes_per_sec_2 > 1000 * 1000 * 100)
std::cerr<<"Warning: trying to transmit > 100Mb/s"<<std::endl;
if (mode == "boost")
{
UdpSendTest u(io_service,
argv[2],dest_port, packet_size, packet_count, frame_interval);
u.start();
io_service.run();
}
else
{
UdpSendTestLowlevel u(io_service,
argv[2],dest_port, packet_size, packet_count, frame_interval);
u.start();
io_service.run();
}
return 0;
}
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH] cpsw: ethtool: add support for getting/setting EEE registers
From: yegorslists @ 2016-11-23 10:34 UTC (permalink / raw)
To: netdev; +Cc: linux-omap, grygorii.strashko, mugunthanvnm, Yegor Yefremov
From: Yegor Yefremov <yegorslists@googlemail.com>
Add the ability to query and set Energy Efficient Ethernet parameters
via ethtool for applicable devices.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
drivers/net/ethernet/ti/cpsw.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index c6cff3d..6856616 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2239,6 +2239,30 @@ static int cpsw_set_channels(struct net_device *ndev,
return ret;
}
+int cpsw_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
+{
+ struct cpsw_priv *priv = netdev_priv(ndev);
+ struct cpsw_common *cpsw = priv->cpsw;
+ int slave_no = cpsw_slave_index(cpsw, priv);
+
+ if (cpsw->slaves[slave_no].phy)
+ return phy_ethtool_get_eee(cpsw->slaves[slave_no].phy, edata);
+ else
+ return -EOPNOTSUPP;
+}
+
+int cpsw_set_eee(struct net_device *ndev, struct ethtool_eee *edata)
+{
+ struct cpsw_priv *priv = netdev_priv(ndev);
+ struct cpsw_common *cpsw = priv->cpsw;
+ int slave_no = cpsw_slave_index(cpsw, priv);
+
+ if (cpsw->slaves[slave_no].phy)
+ return phy_ethtool_set_eee(cpsw->slaves[slave_no].phy, edata);
+ else
+ return -EOPNOTSUPP;
+}
+
static const struct ethtool_ops cpsw_ethtool_ops = {
.get_drvinfo = cpsw_get_drvinfo,
.get_msglevel = cpsw_get_msglevel,
@@ -2262,6 +2286,8 @@ static const struct ethtool_ops cpsw_ethtool_ops = {
.complete = cpsw_ethtool_op_complete,
.get_channels = cpsw_get_channels,
.set_channels = cpsw_set_channels,
+ .get_eee = cpsw_get_eee,
+ .set_eee = cpsw_set_eee,
};
static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_common *cpsw,
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Arnd Bergmann @ 2016-11-23 10:15 UTC (permalink / raw)
To: Jisheng Zhang
Cc: linux-arm-kernel, Thomas Petazzoni, Andrew Lunn, Jason Cooper,
netdev, linux-kernel, Gregory CLEMENT, Marcin Wojtas,
David S. Miller, Sebastian Hesselbarth
In-Reply-To: <20161123175341.4777595f@xhacker>
On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote:
> On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote:
>
> > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote:
> > > +#ifdef CONFIG_64BIT
> > > + void *data_tmp;
> > > +
> > > + /* In Neta HW only 32 bits data is supported, so in order to
> > > + * obtain whole 64 bits address from RX descriptor, we store
> > > + * the upper 32 bits when allocating buffer, and put it back
> > > + * when using buffer cookie for accessing packet in memory.
> > > + * Frags should be allocated from single 'memory' region,
> > > + * hence common upper address half should be sufficient.
> > > + */
> > > + data_tmp = mvneta_frag_alloc(pp->frag_size);
> > > + if (data_tmp) {
> > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32;
> > > + mvneta_frag_free(pp->frag_size, data_tmp);
> > > + }
> > >
> >
> > How does this work when the region spans a n*4GB address boundary?
>
> indeed. We also make use of this driver on 64bit platforms. We use
> different solution to make the driver 64bit safe.
>
> solA: make use of the reserved field in the mvneta_rx_desc, such
> as reserved2 etc. Yes, the field is marked as "for future use, PnC", but
> now it's not used at all. This is one possible solution however.
Right, this sounds like the most straightforward choice.
> solB: allocate a shadow buf cookie during init, e.g
>
> rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL);
>
> then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in
> the shadow buf cookie, e.g
> static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
> u32 phys_addr, u32 cookie,
> struct mvneta_rx_queue *rxq)
>
> {
> int i;
>
> rx_desc->buf_cookie = cookie;
> rx_desc->buf_phys_addr = phys_addr;
> i = rx_desc - rxq->descs;
> rxq->descs_bufcookie[i] = cookie;
> }
>
> then fetch the desc from the shadow buf cookie in all code path, such
> as mvneta_rx() etc.
>
> Both solutions should not have the problems pointed out by Arnd.
Wait, since you compute an index 'i' here, can't you just store 'i'
directly in the descriptor instead of the pointer?
Arnd
^ permalink raw reply
* Re: [PATCH net-next 1/2] samples/bpf: fix sockex2 example
From: Daniel Borkmann @ 2016-11-23 9:58 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <1479862329-2361912-1-git-send-email-ast@fb.com>
On 11/23/2016 01:52 AM, Alexei Starovoitov wrote:
> since llvm commit "Do not expand UNDEF SDNode during insn selection lowering"
> llvm will generate code that uses uninitialized registers for cases
> where C code is actually uses uninitialized data.
> So this sockex2 example is technically broken.
> Fix it by initializing on the stack variable fully.
> Also increase verifier buffer limit, since verifier output
> may not fit in 64k for this sockex2 code depending on llvm version.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next 2/2] samples/bpf: fix bpf loader
From: Daniel Borkmann @ 2016-11-23 9:57 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <1479862329-2361912-2-git-send-email-ast@fb.com>
On 11/23/2016 01:52 AM, Alexei Starovoitov wrote:
> llvm can emit relocations into sections other than program code
> (like debug info sections). Ignore them during parsing of elf file
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: egress all frames
From: Stefan Eichenberger @ 2016-11-23 9:56 UTC (permalink / raw)
To: Vivien Didelot; +Cc: Andrew Lunn, Stefan Eichenberger, f.fainelli, netdev
In-Reply-To: <87zikr40w2.fsf@ketchup.i-did-not-set--mail-host-address--so-tickle-me>
Hi Vivien
On Tue, Nov 22, 2016 at 05:15:25PM -0500, Vivien Didelot wrote:
> Hi Andrew, Stefan,
>
> Andrew Lunn <andrew@lunn.ch> writes:
>
> > What you might find useful is
> >
> > https://github.com/vivien/linux.git 161b96bd7d16d21b0f046c935b70c3b2d277ccc2
> >
> > although it might need some changes for recent commits.
> >
> > With that, you can see deeper into the switches registers.
>
> FYI, I have rebased it on top of the latest net-next (f9aa9dc7d2d0):
>
> https://github.com/vivien/linux.git dsa/dev
>
Perfect that is really helpful, thanks a lot!
Stefan
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Jisheng Zhang @ 2016-11-23 9:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Thomas Petazzoni, Andrew Lunn, Jason Cooper, netdev, linux-kernel,
Gregory CLEMENT, Marcin Wojtas, David S. Miller, linux-arm-kernel,
Sebastian Hesselbarth
In-Reply-To: <2948812.F3se4ieqO6@wuerfel>
On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote:
> > +#ifdef CONFIG_64BIT
> > + void *data_tmp;
> > +
> > + /* In Neta HW only 32 bits data is supported, so in order to
> > + * obtain whole 64 bits address from RX descriptor, we store
> > + * the upper 32 bits when allocating buffer, and put it back
> > + * when using buffer cookie for accessing packet in memory.
> > + * Frags should be allocated from single 'memory' region,
> > + * hence common upper address half should be sufficient.
> > + */
> > + data_tmp = mvneta_frag_alloc(pp->frag_size);
> > + if (data_tmp) {
> > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32;
> > + mvneta_frag_free(pp->frag_size, data_tmp);
> > + }
> >
>
> How does this work when the region spans a n*4GB address boundary?
indeed. We also make use of this driver on 64bit platforms. We use
different solution to make the driver 64bit safe.
solA: make use of the reserved field in the mvneta_rx_desc, such
as reserved2 etc. Yes, the field is marked as "for future use, PnC", but
now it's not used at all. This is one possible solution however.
solB: allocate a shadow buf cookie during init, e.g
rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL);
then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in
the shadow buf cookie, e.g
static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
u32 phys_addr, u32 cookie,
struct mvneta_rx_queue *rxq)
{
int i;
rx_desc->buf_cookie = cookie;
rx_desc->buf_phys_addr = phys_addr;
i = rx_desc - rxq->descs;
rxq->descs_bufcookie[i] = cookie;
}
then fetch the desc from the shadow buf cookie in all code path, such
as mvneta_rx() etc.
Both solutions should not have the problems pointed out by Arnd.
Thanks,
Jisheng
^ permalink raw reply
* Re: [PATCH net-next 1/1] ipv6: sr: add option to control lwtunnel support
From: David Lebrun @ 2016-11-23 9:28 UTC (permalink / raw)
To: Roopa Prabhu, Alexei Starovoitov
Cc: David Miller, netdev@vger.kernel.org, Lorenzo Colitti,
Eric Dumazet
In-Reply-To: <5835466B.6080405@cumulusnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]
On 11/23/2016 08:34 AM, Roopa Prabhu wrote:
> I can't seem to reproduce the problem you are seeing. still trying..
> I don't have CONFIG_LWTUNNEL set nor any of the other SEG6 configs.
> My CONFIG_IPV6 is on and compiled as a module. I have also tried disabling it.
> If you can send me the config, I can try again. Looking back at the patches,
> I do see a few things below ..but they may not fix your problem directly.
>
> Though I had none of the ipv6 segment routing configs turned on,
> I do see the "Segment Routing with IPv6" msg at bootup.
> Was looking at david's patches again, and a few things (I had missed seeing the last version):
>
> In my review comment I was hinting at CONFIG_IPV6_SEG6 to cover all of ipv6 segment routing,
> including the lwtunnel bits.
>
> something like below:
>
> config IPV6_SEG6
> bool "IPv6: Segment Routing Header encapsulation support"
> depends on LWTUNNEL && IPV6
>
> DavidL, do you see a problem doing it this way ?. with this 'seg6.o' will be part of CONFIG_IPV6_SEG6 and not
> get initialized unless it is enabled..which seems like the right thing to do.
Can't reproduce the bug either, with CONFIG_IPV6=y, LWTUNNEL=n and all
SEG6 disabled. Alexei, your .config and dmesg log could help.
Roopa, the reason why seg6.o is compiled by default is that it provides
an interface to control HMAC structures, and that HMAC does not depends
on lwtunnels and can be used in the extension header processing (which
is compiled by default). I could indeed add another option to
conditionnally compile seg6.o if HMAC is enabled etc, and I actually had
something like that in the very first versions of the patch, but I
received comments that too much options is not a good thing (and I agree
with that).
Anyway, I do not see how seg6.o could possibly generate such a bug given
the only thing it does is register a genetlink family and pernet ops
that allocate/deallocate a struct. Genetlink is compiled by default with
NET and register_pernet_subsys does not fail even when namespaces
support is disabled.
David
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]
^ permalink raw reply
* Re: sendfile from 9p fs into af_alg
From: Alexei Starovoitov @ 2016-11-23 8:58 UTC (permalink / raw)
To: Al Viro; +Cc: linux-kernel, netdev, Daniel Borkmann, Martin KaFai Lau
In-Reply-To: <20161123061628.GN1555@ZenIV.linux.org.uk>
On Wed, Nov 23, 2016 at 06:16:28AM +0000, Al Viro wrote:
> On Tue, Nov 22, 2016 at 08:55:59PM -0800, Alexei Starovoitov wrote:
> > On Wed, Nov 23, 2016 at 04:46:26AM +0000, Al Viro wrote:
> > > On Tue, Nov 22, 2016 at 07:58:29PM -0800, Alexei Starovoitov wrote:
> > > > Hi Al,
> > > >
> > > > it seems the following commit 523ac9afc73a ("switch default_file_splice_read() to use of pipe-backed iov_iter")
> > > > breaks sendfile from 9p fs into af_alg socket.
> > > > sendfile into af_alg is used by iproute2/tc.
> > > > I'm not sure whether it's 9p or crypto or vfs problem, but happy to test any patches.
> > >
> > > Could you try -rc6 (or anything that contains 680bb946a1ae04, for that
> > > matter)?
> >
> > already tested with that patch in the latest net-next. Still broken :(
>
> Joy... Which transport are you using there? The interesting part is
> whether it's zerocopy or non-zerocopy path in p9_client_read()...
not sure what's the default is. It's a standard qemu setup:
sudo /usr/bin/qemu-system-x86_64 -enable-kvm -smp 4 -cpu host \
-kernel .../bld_x64/arch/x86/boot/bzImage \
-drive file=....qcow2,if=virtio \
-no-reboot -m 4096 \
--append "root=/dev/vda1 rw mem=GG vga=0 console=ttyS0" -nographic \
-fsdev local,security_model=passthrough,id=fsdev1,path=/data/users \
-device virtio-9p-pci,id=fs1,fsdev=fsdev1,mount_tag=hostshare1
Enabled CONFIG_NET_9P_DEBUG and everything looks normal:
# ./a.out ./a.out
[ 23.894140] 9pnet: -- v9fs_vfs_lookup (235): dir: ffff8801370d87f8 dentry: (a.out) ffff880139ffe600 flags: 0
[ 23.895409] 9pnet: -- v9fs_fid_find (235): dentry: bpf (ffff880139ffc180) uid 0 any 0
[ 23.896451] 9pnet: -- p9_fid_create (235): clnt ffff880136d8f000
[ 23.897225] 9pnet: -- p9_idpool_get (235): id 6 pool ffff880139b76640
[ 23.898052] 9pnet: (00000235) >>> TWALK fids 5,6 nwname 1d wname[0] a.out
[ 23.898919] 9pnet: -- p9_client_prepare_req (235): client ffff880136d8f000 op 110
[ 23.899884] 9pnet: -- p9_idpool_get (235): id 1 pool ffff880139b76c00
[ 23.900738] 9pnet: (00000235) >>> size=24 type: 110 tag: 1
[ 23.901452] 9pnet: -- p9_virtio_request (235): 9p debug: virtio request
[ 23.902332] 9pnet: -- p9_virtio_request (235): virtio request kicked
[ 23.903374] 9pnet: -- req_done (235): : request done
[ 23.903377] 9pnet: -- p9_client_cb (235): tag 1
[ 23.903378] 9pnet: -- p9_client_cb (235): wakeup: 1
[ 23.905213] 9pnet: (00000235) <<< size=22 type: 111 tag: 1
[ 23.905904] 9pnet: -- p9_free_req (235): clnt ffff880136d8f000 req ffff880138eac070 tag: 1
[ 23.906943] 9pnet: -- p9_idpool_put (235): id 1 pool ffff880139b76c00
[ 23.907847] 9pnet: (00000235) <<< RWALK nwqid 1:
[ 23.908446] 9pnet: (00000235) <<< [0] 0.170dd824.58117466
[ 23.909184] 9pnet: (00000235) >>> TGETATTR fid 6, request_mask 6143
[ 23.909980] 9pnet: -- p9_client_prepare_req (235): client ffff880136d8f000 op 24
[ 23.910887] 9pnet: -- p9_idpool_get (235): id 1 pool ffff880139b76c00
[ 23.911737] 9pnet: (00000235) >>> size=19 type: 24 tag: 1
[ 23.912426] 9pnet: -- p9_virtio_request (235): 9p debug: virtio request
[ 23.913266] 9pnet: -- p9_virtio_request (235): virtio request kicked
[ 23.914159] 9pnet: -- req_done (235): : request done
[ 23.914161] 9pnet: -- p9_client_cb (235): tag 1
[ 23.914162] 9pnet: -- p9_client_cb (235): wakeup: 1
[ 23.915982] 9pnet: (00000235) <<< size=160 type: 25 tag: 1
[ 23.916691] 9pnet: (00000235) <<< RGETATTR st_result_mask=6143
<<< qid=0.170dd824.58117466
<<< st_mode=000081ed st_nlink=1
<<< st_uid=572438 st_gid=100
<<< st_rdev=0 st_size=2598 st_blksize=4096 st_blocks=24
<<< st_atime_sec=1479863398 st_atime_nsec=904285549
<<< st_mtime_sec=1479863398 st_mtime_nsec=914285509
<<< st_ctime_sec=1479863398 st_ctime_nsec=914285509
<<< st_btime_sec=0 st_btime_nsec=0
<<< st_gen=1570962252 st_data_version=0[ 23.921484] 9pnet: -- p9_free_req (235): clnt ffff880136d8f000 req ffff880138eac070 tag: 1
[ 23.922536] 9pnet: -- p9_idpool_put (235): id 1 pool ffff880139b76c00
[ 23.923368] 9pnet: -- v9fs_file_open (235): inode: ffff8801370d0568 file: ffff88013a566500
[ 23.924451] 9pnet: -- v9fs_fid_find (235): dentry: a.out (ffff880139ffe600) uid 0 any 0
[ 23.925483] 9pnet: -- p9_fid_create (235): clnt ffff880136d8f000
[ 23.926263] 9pnet: -- p9_idpool_get (235): id 7 pool ffff880139b76640
---skip---
[ 24.044275] 9pnet: -- req_done (123): : request done
[ 24.044278] 9pnet: -- p9_client_cb (123): tag 1
[ 24.044278] 9pnet: -- p9_client_cb (123): wakeup: 1
[ 24.047135] 9pnet: (00000235) <<< size=4107 type: 117 tag: 1
[ 24.047879] 9pnet: (00000235) <<< RREAD count 4096
[ 24.048520] 9pnet: -- p9_free_req (235): clnt ffff880136d8f000 req ffff880138eac070 tag: 1
[ 24.049609] 9pnet: -- p9_idpool_put (235): id 1 pool ffff880139b76c00
[ 24.050462] 9pnet: -- p9_client_prepare_req (235): client ffff880136d8f000 op 116
[ 24.051431] 9pnet: -- p9_idpool_get (235): id 1 pool ffff880139b76c00
[ 24.052283] 9pnet: (00000235) >>> size=23 type: 116 tag: 1
[ 24.052984] 9pnet: -- p9_virtio_zc_request (235): virtio request
[ 24.053774] 9pnet: -- p9_virtio_zc_request (235): virtio request kicked
[ 24.053834] 9pnet: -- req_done (123): : request done
[ 24.053836] 9pnet: -- p9_client_cb (123): tag 1
[ 24.053836] 9pnet: -- p9_client_cb (123): wakeup: 1
[ 24.056496] 9pnet: (00000235) <<< size=4107 type: 117 tag: 1
[ 24.057211] 9pnet: (00000235) <<< RREAD count 4096
[ 24.057820] 9pnet: -- p9_free_req (235): clnt ffff880136d8f000 req ffff880138eac070 tag: 1
[ 24.058857] 9pnet: -- p9_idpool_put (235): id 1 pool ffff880139b76c00
[ 24.059800] 9pnet: -- v9fs_dir_release (235): inode: ffff8801370d0568 filp: ffff880139ab2800 fid: 8
Error from sendf[ 24.060938] 9pnet: (00000235) >>> TCLUNK fid 8 (try 0)
ile (8192 vs 962[ 24.061731] 9pnet: -- p9_client_prepare_req (235): client ffff880136d8f000 op 120
4 bytes): Succes[ 24.062787] 9pnet: -- p9_idpool_get (235): id 1 pool ffff880139b76c00
s
[ 24.063715] 9pnet: (00000235) >>> size=11 type: 120 tag: 1
[ 24.064461] 9pnet: -- p9_virtio_request (235): 9p debug: virtio request
[ 24.065335] 9pnet: -- p9_virtio_request (235): virtio request kicked
[ 24.065410] 9pnet: -- req_done (0): : request done
[ 24.065412] 9pnet: -- p9_client_cb (0): tag 1
[ 24.065413] 9pnet: -- p9_client_cb (0): wakeup: 1
[ 24.068025] 9pnet: (00000235) <<< size=7 type: 121 tag: 1
[ 24.068695] 9pnet: (00000235) <<< RCLUNK fid 8
[ 24.069253] 9pnet: -- p9_free_req (235): clnt ffff880136d8f000 req ffff880138eac070 tag: 1
[ 24.070269] 9pnet: -- p9_idpool_put (235): id 1 pool ffff880139b76c00
[ 24.071120] 9pnet: -- p9_fid_destroy (235): fid 8
[ 24.071735] 9pnet: -- p9_idpool_put (235): id 8 pool ffff880139b76640
hash 0
if I read it correctly 9p actually responded with 8192 bytes of requests...
whereas the file size was 9624.
For large file sizes (in megabytes) the difference between what
sendfile is reporting and actual file size can be 3x.
In the small file case (like above dump) it looks rounded to page size for some reason.
^ permalink raw reply
* Re: [LKP] [net] 34fad54c25: kernel BUG at include/linux/skbuff.h:1935!
From: Ye Xiaolong @ 2016-11-23 8:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Fengguang Wu, David Miller, Eric Dumazet, Alexander Duyck,
Willem de Bruijn, Network Development, LKML, Alexei Starovoitov,
LKP
In-Reply-To: <CA+55aFx9q2xi1oi2j5QcYhMV490oj9CQ4N_OEXzC-3b6GeUQug@mail.gmail.com>
On 11/22, Linus Torvalds wrote:
>On Tue, Nov 22, 2016 at 10:44 PM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>>
>> On Tue, Nov 22, 2016 at 02:04:42PM -0800, Linus Torvalds wrote:
>>
>>> I also noticed that the kernel test robot had screwed up the
>>> participants list for some reason, and had
>>>
>>> "Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>, David S.
>>> Miller" <davem@davemloft.net>
>>>
>>> as one of the participants. So there's some odd commit parsing issue
>>> there somewhere. But Alexander seems to have seen this report despite
>>> that, it just never went anywhere that I can tell.
>>
>>
>> Yeah the robot will CC all "Acked-by" people in the bug reports.
>>
>> Shall we limit it to the below TO/CC list?
>
>No. We do want to keep the Acked-by's on the cc.
>
>But you missed the real problem.
>
>It *didn't* cc the acked-by. Look closer. What happened was that it cc'd this:
>
> "Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>, David S. Miller"
>
> <davem@davemloft.net>
>
Seems that the robot failed to parse the commit log correctly due to
the "Reported-by: xxx" line missed '>' in the end, the robot got fooled
by it and generated wrong result, we'll try to improve it to handle this
kind of case.
net: __skb_flow_dissect() must cap its return value
After Tom patch, thoff field could point past the end of the buffer,
this could fool some callers.
If an skb was provided, skb->len should be the upper limit.
If not, hlen is supposed to be the upper limit.
Fixes: a6e544b0a88b ("flow_dissector: Jump to exit code in __skb_flow_dissect")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Yibin Yang <yibyang@cisco.com
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-by: Willem de Bruijn <willemb@google.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Thanks,
Xiaolong
>ie there is only _one_ email address (that of davem@davemloft.net),
>and the whole "Acked-by: Alexander Duyck <...>" part is quoted as the
>_name_ of that email address.
>
>At least that's what the headers look like for me in the original report:
>
> From: kernel test robot <xiaolong.ye@intel.com>
> To: Eric Dumazet <edumazet@google.com>
> Cc: lkp@01.org, Linus Torvalds <torvalds@linux-foundation.org>,
>LKML <linux-kernel@vger.kernel.org>, Alexei Starovoitov
><ast@kernel.org>, Willem de Bruijn <willemb@google.com>, "Acked-by:
>Alexander Duyck <alexander.h.duyck@intel.com>, David S. Miller"
><davem@davemloft.net>
>
>Notice the quoting of that last "name".
>
> Linus
^ permalink raw reply
* Re: [LKP] [net] 34fad54c25: kernel BUG at include/linux/skbuff.h:1935!
From: Fengguang Wu @ 2016-11-23 8:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: kernel test robot, David Miller, Eric Dumazet, Alexander Duyck,
Willem de Bruijn, Network Development, LKML, Alexei Starovoitov,
LKP
In-Reply-To: <CA+55aFx9q2xi1oi2j5QcYhMV490oj9CQ4N_OEXzC-3b6GeUQug@mail.gmail.com>
On Tue, Nov 22, 2016 at 11:07:16PM -0800, Linus Torvalds wrote:
>On Tue, Nov 22, 2016 at 10:44 PM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>>
>> On Tue, Nov 22, 2016 at 02:04:42PM -0800, Linus Torvalds wrote:
>>
>>> I also noticed that the kernel test robot had screwed up the
>>> participants list for some reason, and had
>>>
>>> "Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>, David S.
>>> Miller" <davem@davemloft.net>
>>>
>>> as one of the participants. So there's some odd commit parsing issue
>>> there somewhere. But Alexander seems to have seen this report despite
>>> that, it just never went anywhere that I can tell.
>>
>>
>> Yeah the robot will CC all "Acked-by" people in the bug reports.
>>
>> Shall we limit it to the below TO/CC list?
>
>No. We do want to keep the Acked-by's on the cc.
>
>But you missed the real problem.
>
>It *didn't* cc the acked-by. Look closer. What happened was that it cc'd this:
>
> "Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>, David S. Miller"
>
> <davem@davemloft.net>
>
>ie there is only _one_ email address (that of davem@davemloft.net),
>and the whole "Acked-by: Alexander Duyck <...>" part is quoted as the
>_name_ of that email address.
>
>At least that's what the headers look like for me in the original report:
>
> From: kernel test robot <xiaolong.ye@intel.com>
> To: Eric Dumazet <edumazet@google.com>
> Cc: lkp@01.org, Linus Torvalds <torvalds@linux-foundation.org>,
>LKML <linux-kernel@vger.kernel.org>, Alexei Starovoitov
><ast@kernel.org>, Willem de Bruijn <willemb@google.com>, "Acked-by:
>Alexander Duyck <alexander.h.duyck@intel.com>, David S. Miller"
><davem@davemloft.net>
>
>Notice the quoting of that last "name".
Ah thanks! Xiaolong just root caused the parse error and will fix it.
Interestingly we didn't see that problem -- the CC list looks correct
in our emails -- perhaps Intel's email system auto fixed up the header.
Thanks,
Fengguang
^ permalink raw reply
* Re: net/arp: ARP cache aging failed.
From: Julian Anastasov @ 2016-11-23 8:33 UTC (permalink / raw)
To: yuehaibing; +Cc: davem, netdev
In-Reply-To: <957c2a80-7302-1ce9-726e-1e7512a941f4@huawei.com>
Hello,
On Wed, 23 Nov 2016, yuehaibing wrote:
> As to my topo,HOST1 and HOST3 share one route on HOST2, tcp connection between HOST2 and HOST3 may call tcp_ack to set dst->pending_confirm.
>
> So dst_neigh_output may wrongly freshed n->confirmed which stands for HOST1,however HOST1'MAC had been changed.
>
> The possibility of this occurred Significantly increases ,when ping and TCP transaction are set the same processor affinity on the HOST2.
>
> It seems that the issue is brought in commit 5110effee8fde2edfacac9cd12a9960ab2dc39ea ("net: Do delayed neigh confirmation.").
Bad news. Problem is not in delayed confirmation but
in the mechanism to use same dst for different neighbours on
LAN. We don't have a dst->neighbour reference anymore.
For IPv4 this is related to rt->rt_uses_gateway but
also to DST_NOCACHE. In the other cases we can not call
dst_confirm, may be we should lookup the neigh entry instead.
But we need a way to reduce such lookups on every packet,
for example, by remembering in struct sock and checking if
some bits of jiffies (at least 4-5) are changed from
previous lookup.
Regards
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Arend Van Spriel @ 2016-11-23 8:24 UTC (permalink / raw)
To: Pali Rohár, Michal Kazior
Cc: Kalle Valo, Pavel Machek, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <201611221805.13606@pali>
On 22-11-2016 18:05, Pali Rohár wrote:
> On Tuesday 22 November 2016 17:14:28 Michal Kazior wrote:
>> On 22 November 2016 at 16:31, Pali Rohár <pali.rohar@gmail.com> wrote:
>>> On Tuesday 22 November 2016 16:22:57 Michal Kazior wrote:
>>>> On 21 November 2016 at 16:51, Pali Rohár <pali.rohar@gmail.com>
>>>> wrote:
>>>>> On Friday 11 November 2016 18:20:50 Pali Rohár wrote:
>>>>>> Hi! I will open discussion about mac address and calibration
>>>>>> data for wl1251 wireless chip again...
>>>>>>
>>>>>> Problem: Mac address & calibration data for wl1251 chip on
>>>>>> Nokia N900 are stored on second nand partition (mtd1) in
>>>>>> special proprietary format which is used only for Nokia N900
>>>>>> (probably on N8x0 and N9 too). Wireless driver wl1251.ko
>>>>>> cannot work without mac address and calibration data.
>>>>
>>>> Same problem applies to some ath9k/ath10k supported routers. Some
>>>> even carry mac address as implicit offset from ethernet mac
>>>> address. As far as I understand OpenWRT cooks cal blobs on first
>>>> boot prior to loading modules.
>>>
>>> So... wl1251 on Nokia N900 is not alone and this problem is there
>>> for more drivers and devices. Which means we should come up with
>>> some generic solution.
>>
>> This isn't particularly a problem for ath9k/ath10k.
>>
>> Let me give you more background on ath10k.
>>
>> ath10k devices can come with caldata and macaddr stored in their
>> OTP/EEPROM. In that case a generic "template" board file is used.
>> Userspace doesn't need to do anything special.
>>
>> Some vendors however decide to use flash partition to store caldata.
>> In that case ath10k expects userspace to prepare
>> cal-$bus-$devname.bin files, each for a different radio (you can
>> have multiple radios on a system).
>>
>> Now translating this for wl1251 I would expect it should also use
>> something like wl1251-nvs-sdio-0x0001.bin for devices like N900 that
>> have caldata on flash partition (instead of the generic
>> wl1251-nvs.bin). I'm not sure if wl1251-nvs.bin is something
>> comparable to (the generic) board.bin ath10k has though. Maybe the
>> entire idea behind wl1251-nvs.bin is flawed as it's supposed to be
>> device specific and is oblivious to possibility of having multiple
>> wl1251 radios on one system (probably sane assumption from practical
>> standpoint but still).
>
> Basically nvs data are device specific, in ideal case they should be
> generated in factory by some calibration process (or so).
For brcmfmac we have what we call nvram data, which is determined during
manufacturing. We use the firmware_class API to obtain that file, but on
router it may be stored in flash. So an API was created for that router
architecture and brcmfmac calls that API [1]. Not a generic solution but
it gets the job done. Personally, I would have liked this to be handled
behind the firmware_class API to hide the storage details from the driver.
Regards,
Arend
[1]
http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c#L449
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox