* Re: [PATCH net-next v4 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jiri Pirko @ 2019-02-26 8:57 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-6-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 04:34:06AM CET, jakub.kicinski@netronome.com wrote:
>When ethtool is calling into devlink compat code make sure we have
>a reference on the netdevice on which the operation was invoked.
>
>v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v4 6/6] devlink: require non-NULL ops for devlink instances
From: Jiri Pirko @ 2019-02-26 8:59 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-7-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 04:34:07AM CET, jakub.kicinski@netronome.com wrote:
>Commit 76726ccb7f46 ("devlink: add flash update command") and
>commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
>access devlink ops without NULL-checking. There is, however, no
>driver which would pass in NULL ops, so let's just make that
>a requirement. Remove the now unnecessary NULL-checking.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Thanks Kuba!
^ permalink raw reply
* Re: [PATCH v4 0/3] Add quirk for reading BD_ADDR from fwnode property
From: Marcel Holtmann @ 2019-02-26 9:09 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Johan Hedberg, David S. Miller, Loic Poulain, linux-bluetooth,
linux-kernel, netdev, Balakrishna Godavarthi
In-Reply-To: <20190219200559.13079-1-mka@chromium.org>
Hi Matthias,
> On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> on the Bluetooth chip itself. One way to configure the address is
> through the device tree (patched in by the bootloader). The btqcomsmd
> driver is an example, it can read the address from the DT property
> 'local-bd-address'.
>
> To avoid redundant open-coded reading of 'local-bd-address' and error
> handling this series adds the quirk HCI_QUIRK_USE_BDADDR_PROPERTY to
> retrieve the BD address of a device from the DT and adapts the
> btqcomsmd and hci_qca drivers to use this quirk.
>
> Matthias Kaehlcke (3):
> Bluetooth: Add quirk for reading BD_ADDR from fwnode property
> Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
> Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990
>
> drivers/bluetooth/btqcomsmd.c | 31 +++----------------------
> drivers/bluetooth/hci_qca.c | 1 +
> include/net/bluetooth/hci.h | 12 ++++++++++
> net/bluetooth/hci_core.c | 43 +++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c | 6 +++--
> 5 files changed, 63 insertions(+), 30 deletions(-)
all 3 patches have been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx buffer
From: Ioana Ciornei @ 2019-02-26 9:14 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Ioana Ciocoi Radulescu
Cc: Ilias Apalodimas, netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <VI1PR0402MB28001EBE94356BFF7C250C86E0690@VI1PR0402MB2800.eurprd04.prod.outlook.com>
> Subject: RE: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx buffer
>
> > Subject: Re: [PATCH net-next 1/4] dpaa2-eth: Use a single page per Rx
> > buffer
> >
> >
> > On Wed, 6 Feb 2019 15:36:33 +0000 Ioana Ciocoi Radulescu
> > <ruxandra.radulescu@nxp.com> wrote:
> >
> > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >
> > > > Can you share any results on XDP (XDP_DROP is usually useful for
> > > > the hardware capabilities).
> > >
> > > XDP numbers are pretty much the same as before this patch:
> > >
> > > On a LS2088A with A72 cores @2GHz (numbers in Mpps):
> > > 1core 8cores
> > > -------------------------------------------------------------------------
> > > XDP_DROP (no touching data) 5.37 29.6 (linerate)
> > > XDP_DROP (xdp1 sample) 3.14 24.22
> >
> > It is interesting/problematic to see that the cost of touching the
> > data is so high 5.37Mpps -> 3.14Mpps. The Intel CPUs have solved this
> > in hardware with DDIO, which delivers frame in L3-cache. I have some
> > ideas on how to improve this on ARM (or CPUs without DDIO). I've
> > previous implemented this as RFC on mlx4 tested on a CPU without DDIO,
> > with great success 10mpps -> 20Mpps (but it was shutdown, as newer
> > Intel HW solved the issue). The basic idea is to have an array of
> > frames, that you start an L2/L3-prefetch on, before going "back" and
> > process them for XDP or netstack. (p.s. this is the same DPDK does)
>
> Thanks for the hint. We are currently investigating what our options are.
> We'll come back with a more detailed answer once we figure out.
>
Hi,
We have enabled cache stashing of frame data in our solution and sent the patches upstream: https://lkml.org/lkml/2019/2/23/41.
Some results with the patches applied on top of the net-next tree are below.
LS2088A with A72 cores @2GHz (numbers in Mpps):
1core 8cores
--------------------------------------------------------------------------------------------------------
XDP_DROP (no touching data) 5.37 29.6 (linerate)
XDP_DROP (xdp1 sample, no stashing) 3.28 24.2
XDP_DROP (xdp1 sample, stashing) 4.64 26.85
Thanks a lot for your hint.
Ioana C
^ permalink raw reply
* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Nicolas.Ferre @ 2019-02-26 9:23 UTC (permalink / raw)
To: f.fainelli, pthombar, davem, netdev, andrew, hkallweit1,
linux-kernel, rafalc, piotrs, jank, Claudiu.Beznea
In-Reply-To: <65c93555-f0d2-1139-a4fd-5c6a80acd9bf@gmail.com>
$subject should begin with "net: macb: "
Parshuram,
Sorry but NACK on the series.
David,
This patch series seem pretty intrusive, so I would like that you wait
for my explicit ACK before applying even next versions of it.
More comments below...
On 25/02/2019 at 18:21, Florian Fainelli wrote:
> On 2/25/19 1:11 AM, Parshuram Raju Thombare wrote:
>>> Le 2/22/19 à 12:12 PM, Parshuram Thombare a écrit :
>>>> This patch add support for PCS (for SGMII interface) and 2.5Gbps MAC
>>>> in Cadence ethernet controller driver.
>>>
>>> At a high level you don't seem to be making use of PHYLINK so which 2.5Gbps
>>> interfaces do you actually support?
>>>
>>
>> New ethernet controller have MAC which support 2.5G speed.
>> Also there is addition of PCS and SGMII interface.
>
> I should have asked this more clearly: have you tested with SFP modules
> for instance? If you want to be able to reliably support 2500baseT
> and/or 2500baseX with hot plugging of such modules, you need to
> implement PHYLINK for that network driver, there is no other way around.
>
>>
>>>>
>>>> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
>>>> ---
>>>
[..]
>>>>
>>>> - switch (speed) {
>>>> - case SPEED_10:
>>>> + if (interface == PHY_INTERFACE_MODE_GMII ||
>>>> + interface == PHY_INTERFACE_MODE_MII) {
>>>> + switch (speed) {
>>>> + case SPEED_10:> rate = 2500000;
>>>
>>> You need to add one tab to align rate and break.
>>
>> Do you mean a tab each for rate and break lines ?
>> All switch statements are aligned at a tab. I am not sure how does case and rate got on same line.
>
> It should look like this:
>
> switch (cond) {
> case cond1:
> do_something();
> break;
>
> etc.
Read the coding style documentation, everything is well explained there.
>>>> break;
>>>> - case SPEED_100:
>>>> + case SPEED_100:
>>>> rate = 25000000;
>>>> break;
>>>> - case SPEED_1000:
>>>> + case SPEED_1000:
>>>> rate = 125000000;
>>>> break;
>>>> - default:
>>>> + default:
>>>> + return;
>>>> + }
>>>> + } else if (interface == PHY_INTERFACE_MODE_SGMII) {
>>>> + switch (speed) {
>>>> + case SPEED_10:
>>>> + rate = 1250000;
>>>> + break;
>>>> + case SPEED_100:
>>>> + rate = 12500000;
>>>> + break;
>>>> + case SPEED_1000:
>>>> + rate = 125000000;
>>>> + break;
>>>> + case SPEED_2500:
>>>> + rate = 312500000;
>>>> + break;
>>>> + default:
>>>> + return;
>>>
>>> The indentation is broken here and you can greatly simplify this with a simple
>>> function that returns speed * 1250 and does an initial check for unsupported
>>> speeds.
>>>
>>
>> I ran checkpatch.pl and all indentation issues were cleared. But I think having function
>> is better option, I will make that change.
>>
>>>> + }
>>>> + } else {
>>>> return;
>>>> }
[..]
>>>> int macb_mii_probe(struct net_device *dev)
>>>> }
>>>>
>>>> /* mask with MAC supported features */
>>>> - if (macb_is_gem(bp) && bp->caps &
>>> MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>>>> - phy_set_max_speed(phydev, SPEED_1000);
>>>> - else
>>>> - phy_set_max_speed(phydev, SPEED_100);
>>>> + if (macb_is_gem(bp)) {
>>>
>>> You have changed the previous logic that also checked for
>>> MACB_CAPS_GIGABIT_MODE_AVAILABLE, why?
>>
>> My understanding is all GEM (ID >= 0x2) support GIGABIT mode.
>> Was there any other reason for this check ?
We use (ID >= 0x2) and only 10/100 mode on sama5d4/sama5d2 for more than
5 years, as seen in their respective struct macb_config, weren't you aware?
> Well, if anyone would know, it would be you, I don't work for Cadence
> nor have access to the internal IP documentation.
I agree with Florian, and what you modify makes me feel that the rest of
the patch might break my use of the Cadence IP in my products. That's
not a good sign, to say the least.
I'm expecting that Cadence don't break software used by their customers
on products already deployed on the field.
For next series, I would like that you report that you actually tested
your changes on older IP revisions and that non-regression tests are
passed successfully for some of the long time users of this IP (namely
Microchip/Atmel and Xilinx products).
>>>> + linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>>>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>>>> +
>>> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>>>> + phydev->supported);
>>>> + } else {
>>>> + linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>>>> + }
>>>> +
>>>> + linkmode_copy(phydev->advertising, phydev->supported);
>>>>
>>>> if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>>>> phy_remove_link_mode(phydev,
>>>> @@ -2217,8 +2267,6 @@ static void macb_init_hw(struct macb *bp)
>>>> macb_set_hwaddr(bp);
>>>>
>>>> config = macb_mdc_clk_div(bp);
>>>> - if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>>>> - config |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
>>>> config |= MACB_BF(RBOF, NET_IP_ALIGN); /* Make eth data
>>> aligned */
>>>> config |= MACB_BIT(PAE); /* PAuse Enable */
>>>> config |= MACB_BIT(DRFCS); /* Discard Rx FCS */
>>>> @@ -3255,6 +3303,23 @@ static void macb_configure_caps(struct macb *bp,
>>>> dcfg = gem_readl(bp, DCFG1);
>>>> if (GEM_BFEXT(IRQCOR, dcfg) == 0)
>>>> bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>>>> + if (GEM_BFEXT(NO_PCS, dcfg) == 0)
>>>> + bp->caps |= MACB_CAPS_PCS;
>>>> + switch (MACB_BFEXT(IDNUM, macb_readl(bp, MID))) {
>>>> + case MACB_GEM7016_IDNUM:
>>>> + case MACB_GEM7017_IDNUM:
>>>> + case MACB_GEM7017A_IDNUM:
>>>> + case MACB_GEM7020_IDNUM:
>>>> + case MACB_GEM7021_IDNUM:
>>>> + case MACB_GEM7021A_IDNUM:
>>>> + case MACB_GEM7022_IDNUM:
>>>> + if (bp->caps & MACB_CAPS_PCS)
>>>> + bp->caps |= MACB_CAPS_TWO_PT_FIVE_GIG_SPEED;
>>>> + break;
>>>> +
>>>> + default:
>>>> + break;
>>>> + }
>>>> dcfg = gem_readl(bp, DCFG2);
>>>> if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF)))
>>> == 0)
>>>> bp->caps |= MACB_CAPS_FIFO_MODE;
>>>> @@ -4110,7 +4175,28 @@ static int macb_probe(struct platform_device
>>> *pdev)
>>>> else
>>>> bp->phy_interface = PHY_INTERFACE_MODE_MII;
>>>> } else {
>>>> + switch (err) {
>>>> + case PHY_INTERFACE_MODE_SGMII:
>>>> + if (bp->caps & MACB_CAPS_PCS) {
>>>> + bp->phy_interface = PHY_INTERFACE_MODE_SGMII;
>>>> + break;
>>>> + }
>>>
>>> If SGMII was selected on a version of the IP that does not support it, then falling
>>> back to GMII or MII does not sound correct, this is a hard error that must be
>>> handled as such.
>>> --
>>> Florian
>>
>> My intention was to continue (instead of failing) with whatever functionality is available.
>> Can we have some error message and continue with what is available ?
>
> This is probably not going to help anyone, imagine you incorrectly
> specified a 'phy-mode' property in the Device Tree and you have to dig
> into the driver in the function that sets the clock rate to find out
> what you just defaulted to 1Gbits/GMII for instance. This is not user
> friendly at all and will increase the support burden on your end as
> well, if SGMII is specified and the IP does not support it, detect it
> and return an error, how that propagates, either as a probe failure or
> the inability to open the network device, your call.
>
Best regards,
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH net] tcp: repaired skbs must init their tso_segs
From: Andrei Vagin @ 2019-02-26 9:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Soheil Hassas Yeganeh,
Neal Cardwell, Yuchung Cheng, syzbot, Andrey Vagin
In-Reply-To: <20190223235151.168283-1-edumazet@google.com>
On Sat, Feb 23, 2019 at 03:51:51PM -0800, Eric Dumazet wrote:
> syzbot reported a WARN_ON(!tcp_skb_pcount(skb))
> in tcp_send_loss_probe() [1]
>
> This was caused by TCP_REPAIR sent skbs that inadvertenly
> were missing a call to tcp_init_tso_segs()
>
> [1]
> WARNING: CPU: 1 PID: 0 at net/ipv4/tcp_output.c:2534 tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.0.0-rc7+ #77
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> <IRQ>
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> panic+0x2cb/0x65c kernel/panic.c:214
> __warn.cold+0x20/0x45 kernel/panic.c:571
> report_bug+0x263/0x2b0 lib/bug.c:186
> fixup_bug arch/x86/kernel/traps.c:178 [inline]
> fixup_bug arch/x86/kernel/traps.c:173 [inline]
> do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
> do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
> RIP: 0010:tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Code: 88 fc ff ff 4c 89 ef e8 ed 75 c8 fb e9 c8 fc ff ff e8 43 76 c8 fb e9 63 fd ff ff e8 d9 75 c8 fb e9 94 f9 ff ff e8 bf 03 91 fb <0f> 0b e9 7d fa ff ff e8 b3 03 91 fb 0f b6 1d 37 43 7a 03 31 ff 89
> RSP: 0018:ffff8880ae907c60 EFLAGS: 00010206
> RAX: ffff8880a989c340 RBX: 0000000000000000 RCX: ffffffff85dedbdb
> RDX: 0000000000000100 RSI: ffffffff85dee0b1 RDI: 0000000000000005
> RBP: ffff8880ae907c90 R08: ffff8880a989c340 R09: ffffed10147d1ae1
> R10: ffffed10147d1ae0 R11: ffff8880a3e8d703 R12: ffff888091b90040
> R13: ffff8880a3e8d540 R14: 0000000000008000 R15: ffff888091b90860
> tcp_write_timer_handler+0x5c0/0x8a0 net/ipv4/tcp_timer.c:583
> tcp_write_timer+0x10e/0x1d0 net/ipv4/tcp_timer.c:607
> call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
> expire_timers kernel/time/timer.c:1362 [inline]
> __run_timers kernel/time/timer.c:1681 [inline]
> __run_timers kernel/time/timer.c:1649 [inline]
> run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
> __do_softirq+0x266/0x95a kernel/softirq.c:292
> invoke_softirq kernel/softirq.c:373 [inline]
> irq_exit+0x180/0x1d0 kernel/softirq.c:413
> exiting_irq arch/x86/include/asm/apic.h:536 [inline]
> smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
> </IRQ>
> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 0c a1 fa 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 48 0c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
> RSP: 0018:ffff8880a98afd78 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
> RAX: 1ffffffff1125061 RBX: ffff8880a989c340 RCX: 0000000000000000
> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffff8880a989cbbc
> RBP: ffff8880a98afda8 R08: ffff8880a989c340 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
> R13: ffffffff889282f8 R14: 0000000000000001 R15: 0000000000000000
> arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
> default_idle_call+0x36/0x90 kernel/sched/idle.c:93
> cpuidle_idle_call kernel/sched/idle.c:153 [inline]
> do_idle+0x386/0x570 kernel/sched/idle.c:262
> cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
> start_secondary+0x404/0x5c0 arch/x86/kernel/smpboot.c:271
> secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
Thank you Eric. I saw a few test fails when tcp_peek_sndq()
returned more data than we expected. I have executed the test with this
fix in a loop and it works without any problem. Without this fix, it
fails after a few iteration.
https://github.com/checkpoint-restore/criu/issues/622
> Fixes: 79861919b889 ("tcp: fix TCP_REPAIR xmit queue setup")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Andrey Vagin <avagin@openvz.org>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
> net/ipv4/tcp_output.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 730bc44dbad9363814705b28c2f91a2253d91207..ccc78f3a4b60d3012430488bdfbcfc5122ff8627 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2347,6 +2347,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
> /* "skb_mstamp_ns" is used as a start point for the retransmit timer */
> skb->skb_mstamp_ns = tp->tcp_wstamp_ns = tp->tcp_clock_cache;
> list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
> + tcp_init_tso_segs(skb, mss_now);
> goto repair; /* Skip network transmission */
> }
>
> --
> 2.21.0.rc0.258.g878e2cd30e-goog
>
^ permalink raw reply
* [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Julius Niedworok @ 2019-02-26 9:40 UTC (permalink / raw)
To: linux-wireless
Cc: julius.n, ga58taw, david, nc, David S. Miller, Johannes Berg,
Edward Cree, Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
linux-kernel
At Technical University of Munich we use MAC 802.11 TX status frames to
perform several measurements in MAC 802.11 setups.
With ath based drivers this was possible until commit d94a461d7a7df6
("ath9k: use ieee80211_tx_status_noskb where possible") as the driver
ignored the IEEE80211_TX_CTL_REQ_TX_STATUS flag and always delivered
TX status frames. Since this commit, this behavior was changed and the
driver now adheres to IEEE80211_TX_CTL_REQ_TX_STATUS.
Due to performance reasons, IEEE80211_TX_CTL_REQ_TX_STATUS is not set for
data frames from interfaces in managed mode. Hence, frames that are sent
from a managed mode interface do never deliver TX status frames. This
remains true even if a monitor mode interface (e.g. a measurement
interface) is added to the same wireless hardware device. Thus, there is
no possibility for receiving TX status frames for frames sent on an
interface in managed mode if the driver adheres to
IEEE80211_TX_CTL_REQ_TX_STATUS.
In order to force delivery of TX status frames for research and debugging
purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag
is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled
in all packets sent from that interface. IFF_ECHO can be set via
/sys/class/net/<dev>/flags. The default is disabled.
Co-developed-by: Charlie Groh <ga58taw@mytum.de>
Signed-off-by: Charlie Groh <ga58taw@mytum.de>
Signed-off-by: Julius Niedworok <julius.n@gmx.net>
---
net/core/dev.c | 8 ++++++++
net/mac80211/tx.c | 6 ++++++
2 files changed, 14 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 8e276e0..076b556 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7543,6 +7543,14 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags,
IFF_AUTOMEDIA)) |
(dev->flags & (IFF_UP | IFF_VOLATILE | IFF_PROMISC |
IFF_ALLMULTI));
+ /*
+ * Force TX status frames on ieee80211 devices.
+ * Since IFF_ECHO is used by CAN devices for a different
+ * purpose, we must check dev->ieee80211_ptr.
+ */
+
+ if (dev->ieee80211_ptr)
+ dev->flags = (dev->flags & ~IFF_ECHO) | (flags & IFF_ECHO);
/*
* Load in the correct multicast list now the flags have changed.
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 928f13a..2a02f66 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2463,6 +2463,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
if (IS_ERR(sta))
sta = NULL;
+ if (sdata->dev->flags & IFF_ECHO)
+ info_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+
/* convert Ethernet header to proper 802.11 header (based on
* operation mode) */
ethertype = (skb->data[12] << 8) | skb->data[13];
@@ -3468,6 +3471,9 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
(tid_tx ? IEEE80211_TX_CTL_AMPDU : 0);
info->control.flags = IEEE80211_TX_CTRL_FAST_XMIT;
+ if (sdata->dev->flags & IFF_ECHO)
+ info->flags |= IEEE80211_TX_CTL_REQ_TX_STATUS;
+
if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
*ieee80211_get_qos_ctl(hdr) = tid;
--
2.10.1 (Apple Git-78)
^ permalink raw reply related
* [PATCH net-next 1/1] net: sched: act_csum: Fix csum calc for tagged packets
From: Eli Britstein @ 2019-02-26 9:57 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, Jamal Hadi Salim, Cong Wang, Roi Dayan, Maor Dickman,
Eli Britstein
The csum calculation is different for IPv4/6. For VLAN packets,
tc_skb_protocol returns the VLAN protocol rather than the packet's one
(e.g. IPv4/6), so csum is not calculated. Furthermore, VLAN may not be
stripped so csum is not calculated in this case too. Calculate the
csum for those cases.
Fixes: d8b9605d2697 ("net: sched: fix skb->protocol use in case of accelerated vlan path")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/act_csum.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 945fb34ae721..c79aca29505e 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -559,8 +559,11 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
struct tcf_csum *p = to_tcf_csum(a);
+ bool orig_vlan_tag_present = false;
+ unsigned int vlan_hdr_count = 0;
struct tcf_csum_params *params;
u32 update_flags;
+ __be16 protocol;
int action;
params = rcu_dereference_bh(p->params);
@@ -573,7 +576,9 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
goto drop;
update_flags = params->update_flags;
- switch (tc_skb_protocol(skb)) {
+ protocol = tc_skb_protocol(skb);
+again:
+ switch (protocol) {
case cpu_to_be16(ETH_P_IP):
if (!tcf_csum_ipv4(skb, update_flags))
goto drop;
@@ -582,13 +587,35 @@ static int tcf_csum_act(struct sk_buff *skb, const struct tc_action *a,
if (!tcf_csum_ipv6(skb, update_flags))
goto drop;
break;
+ case cpu_to_be16(ETH_P_8021AD): /* fall through */
+ case cpu_to_be16(ETH_P_8021Q):
+ if (skb_vlan_tag_present(skb) && !orig_vlan_tag_present) {
+ protocol = skb->protocol;
+ orig_vlan_tag_present = true;
+ } else {
+ struct vlan_hdr *vlan = (struct vlan_hdr *)skb->data;
+
+ protocol = vlan->h_vlan_encapsulated_proto;
+ skb_pull(skb, VLAN_HLEN);
+ skb_reset_network_header(skb);
+ vlan_hdr_count++;
+ }
+ goto again;
+ }
+
+out:
+ /* Restore the skb for the pulled VLAN tags */
+ while (vlan_hdr_count--) {
+ skb_push(skb, VLAN_HLEN);
+ skb_reset_network_header(skb);
}
return action;
drop:
qstats_drop_inc(this_cpu_ptr(p->common.cpu_qstats));
- return TC_ACT_SHOT;
+ action = TC_ACT_SHOT;
+ goto out;
}
static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
--
2.17.2
^ permalink raw reply related
* [PATCH] net: sched: pie: fix mistake in reference link
From: Leslie Monis @ 2019-02-26 10:23 UTC (permalink / raw)
To: davem; +Cc: netdev, Leslie Monis
Fix the incorrect reference link to RFC 8033
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
net/sched/sch_pie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index f8314a14a256..4c0670b6aec1 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,7 +17,7 @@
* University of Oslo, Norway.
*
* References:
- * RFC 8033: https://tools.ietf.org/html/rfc8034
+ * RFC 8033: https://tools.ietf.org/html/rfc8033
*/
#include <linux/module.h>
--
2.17.1
^ permalink raw reply related
* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Anand Moon @ 2019-02-26 10:25 UTC (permalink / raw)
To: Martin Blumenstingl, Bartosz Golaszewski; +Cc: linux-amlogic, netdev
In-Reply-To: <CANAwSgR=PXtfMJWm_pOLYrV2imKpuw2FQOLT_J73tuu-q7WHww@mail.gmail.com>
Hi Martin,
On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
>
> hi Martin,
>
> +Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > I have seen Anand's your question in [0]:
> > > only issue is I have is the each time their is random MAC address so I
> > > get new IP from dhcp server.
> > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> >
> > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > pass it (via the .dtb) to the kernel.
> > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > for and example.
> >
> > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > controllers: [2] (please note that the function implementing this
> > binding was recently renamed: [3])
> > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > based binding yet
> >
> > Anand, if you want to work on this: feel free to do so!
> > I have the SDHC MMC driver and a discussion about the power-domain
> > drivers on my TODO-list, so I'm pretty busy at the moment.
> >
> >
> > Regards
> > Martin
> >
>
> Thanks for your inputs :) 8)
>
> After enable CONFIG_MESON_MX_EFUSE and added the alias
>
As far as I can tell this nvmem consist of field.
Board ID
MAC address
Serial Number
UID
I feel efues value in nvmem are read with following offset just for testing.
but it also need some driver changes to read from secure memory
by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> On Odroid C1+
> # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> *
> 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000 *00:1e:06:10:dc:ad*
> mac address from nvmem
> 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> 0000200
>
arch/arm/boot/dts/meson8b.dtsi
@@ -360,6 +360,18 @@
compatible = "amlogic,meson8b-efuse";
clocks = <&clkc CLKID_EFUSE>;
clock-names = "core";
+
+ board_sn: board-sn@0000000 {
+ reg = <0x0000000 0x10>;
+ };
+
+ eth_mac: eth-mac@00001b4 {
+ reg = <0x00001b4 0x6>;
+ };
+
+ board_sno: board-sno@00001c0 {
+ reg = <0x00001c0 0x30>;
+ };
This is what I am looking into.
If you have some input please share.
>
> Odroid C2:
> I could clearly observe the same.
>
> [root@archl-c2l alarm]# hexdump /sys/devices/platform/efuse/efuse0/nvmem
> 0000000 0000 0000 0000 0000 0000 0000 0000 0000
> 0000010 0000 0000 4b48 3243 3331 3532 4434 4346
> 0000020 4145 4146 0000 0000 0000 0000 0000 0000
> 0000030 0000 0000 1e00 3306 7a37 0000 0000 0000 *00:1e:06:33:37:7a*
> 0000040 0000 0000 0000 4b48 3243 3132 3631 3430
> 0000050 3834 3932 3631 3837 3937 3466 6233 6366
> 0000060 357c c7ec cc98 0f6d 65b7 92cf 000b 0a00
> 0000070 0000 0000 0000 0000 0000 0000 0000 0000
> 0000080 0014 0000 0000 0000 0000 0000 0000 0000
> 0000090 0000 0000 0000 0000 0000 0000 0000 0000
> *
efuse: efuse {
compatible = "amlogic,meson-gx-efuse",
"amlogic,meson-gxbb-efuse";
#address-cells = <1>;
#size-cells = <1>;
read-only;
sn: sn@14 {
reg = <0x14 0x10>;
};
eth_mac: eth_mac@34 {
reg = <0x34 0x10>;
};
bid: bid@46 {
reg = <0x46 0x30>;
};
};
> >
> > [0] http://lists.infradead.org/pipermail/linux-amlogic/2019-February/010464.html
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7c36209c46c4d162202b65eed2e66962ad8c3c1
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9217e566bdee4583d0a9ea4879c8f5e004886eac
> > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=afa64a72b862a7a9d04f8d07fba632eaf06b23f8
>
> Yes thanks for the inputs.
>
> I see c85efcc60a892210aa10688d5de1f997d5cad799 ("ARM: davinci:
> da830-evm: use cell nvmem lookup for mac address")
> I do not know what this changes will help get the mac address from nvmem region.
>
> If some body could share some inputs it will be better for me to debug.
>
> Best Regards
> -Anand
Best Regards
-Anand
^ permalink raw reply
* Re: [PATCH V2 3/8] dt-bindings: net: stmmac: add phys config properties
From: Christophe ROULLIER @ 2019-02-26 10:30 UTC (permalink / raw)
To: Rob Herring
Cc: davem@davemloft.net, joabreu@synopsys.com, mark.rutland@arm.com,
mcoquelin.stm32@gmail.com, Alexandre TORGUE, Peppe CAVALLARO,
linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
andrew@lunn.ch
In-Reply-To: <20190223001651.GA22381@bogus>
On 2/23/19 1:16 AM, Rob Herring wrote:
> On Fri, Feb 22, 2019 at 09:28:04AM +0100, Christophe Roullier wrote:
>> Add properties to support all Phy config
>> PHY_MODE (MII,GMII, RMII, RGMII) and in normal, PHY wo crystal (25Mhz),
>> PHY wo crystal (50Mhz), No 125Mhz from PHY config.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>> Documentation/devicetree/bindings/net/stm32-dwmac.txt | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> index 1341012..f42dc68 100644
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> @@ -24,9 +24,9 @@ Required properties:
>> encompases the glue register, and the offset of the control register.
>>
>> Optional properties:
>> -- clock-names: For MPU family "mac-clk-ck" for PHY without quartz
>> -- st,int-phyclk (boolean) : valid only where PHY do not have quartz and need to be clock
>> - by RCC
>
> You can't just remove properties.
There is no risk to remove/rename these 2 properties, because it is
specific board which is never deployed.
With new properties (renaming clock (eth-ck) + st,eth_clk_sel and
st,eth_ref_clk_sel, we are managed all kind of specific boards stm32mp1
So no risk of backward compatible.
>
>> +- clock-names: For MPU family "eth-ck" for PHY without quartz
>> +- st,eth_clk_sel (boolean) : set this property in RGMII PHY when you do not want use 125Mhz
>> +- st,eth_ref_clk_sel (boolean) : set this property in RMII mode when you have PHY without crystal 50MHz
>
> s/_/-/
>
> 'sel' I assume is short for select, but the naming here and description
> don't really tell me what I'm getting.
>
Ok, Rob, I will update with your recommendations
> Rob
>
^ permalink raw reply
* Re: [PATCH] net: sched: pie: fix mistake in reference link
From: Leslie Monis @ 2019-02-26 10:34 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <20190226102331.532-1-lesliemonis@gmail.com>
On Tue, Feb 26, 2019 at 03:53:31PM +0530, Leslie Monis wrote:
> Fix the incorrect reference link to RFC 8033
>
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
> ---
> net/sched/sch_pie.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index f8314a14a256..4c0670b6aec1 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -17,7 +17,7 @@
> * University of Oslo, Norway.
> *
> * References:
> - * RFC 8033: https://tools.ietf.org/html/rfc8034
> + * RFC 8033: https://tools.ietf.org/html/rfc8033
> */
>
> #include <linux/module.h>
> --
> 2.17.1
>
I apologize. I forgot to add net-next in the subject prefix.
Leslie
^ permalink raw reply
* [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: Michael Chan @ 2019-02-26 10:56 UTC (permalink / raw)
To: davem, maheshb, edumazet; +Cc: dja, netdev
There have been reports of oversize UDP packets being sent to the
driver to be transmitted, causing error conditions. The issue is
likely caused by the dst of the SKB switching between 'lo' with
64K MTU and the hardware device with a smaller MTU. Patches are
being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
issue.
Separately, we should add a length check in validate_xmit_skb()
to drop these oversize packets before they reach the driver.
This patch only validates non-TSO packets. Complete validation
of segmented TSO packet size will probably be too slow.
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
net/core/dev.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5d03889..50c5174 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3373,6 +3373,13 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
}
}
+ if (!skb_is_gso(skb) &&
+ skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)) {
+ net_warn_ratelimited("%s(): Dropping %d bytes oversize skb.\n",
+ __func__, skb->len);
+ goto out_kfree_skb;
+ }
+
skb = validate_xmit_xfrm(skb, features, again);
return skb;
--
2.5.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-26 11:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jesper Dangaard Brouer, David Miller, netdev, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190225104757.75b622c9@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Sat, 23 Feb 2019 13:11:02 +0100, Toke Høiland-Jørgensen wrote:
>> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>> > On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski wrote:
>> >> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
>> >> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> >> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> > [...]
>> >> > >
>> >> > > BPF programs don't obey by netns boundaries. The fact the program is
>> >> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> >> > > Meaning if any program is using the redirect map you may need a secret
>> >> > > map in every ns.. no?
>> >> >
>> >> > Ah, yes, good point. Totally didn't think about the fact that load and
>> >> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> >> > alloc_default_map() to the point where the program is attached to an
>> >> > interface, then...
>> >>
>> >> Possibly.. and you also need to handle the case where interface with a
>> >> program attached is moved, no?
>>
>> Yup, alloc on attach was easy enough; the moving turns out to be the
>> tricky part :)
>>
>> > True, we need to handle if e.g. a veth gets an XDP program attached and
>> > then is moved into a network namespace (as I've already explained to
>> > Toke in a meeting).
>>
>> Yeah, I had somehow convinced myself that the XDP program was being
>> removed when the interface was being torn down before moving between
>> namespaces. Jesper pointed out that this was not in fact the case... :P
>>
>> > I'm still not sure how to handle this...
>>
>> There are a couple of options, I think. At least:
>>
>> 1. Maintain a flag on struct net_device indicating that this device
>> needs the redirect map allocated, and react to that when interfaces
>> are being moved.
>>
>> 2. Lookup the BPF program by ID (which we can get from the driver) on
>> move, and react to the program flag.
>>
>> 3. Keep the allocation on program load, but allocate maps for all active
>> namespaces (which would probably need a refcnt mechanism to
>> deallocate things again).
>>
>> I think I'm leaning towards #2; possibly combined with a refcnt so we
>> can actually deallocate the map in the root namespace when it's not
>> needed anymore.
>
> Okay.. what about tail calls? I think #3 is most reasonable
> complexity- -wise, or some mix of #2 and #3 - cnt the programs with
> legacy redirects, and then allocate the resources if cnt && name space
> has any XDP program attached.
Yeah, I have that more or less working; except I forgot about tail
calls, but that should not be too difficult to fix.
> Can users really not be told to just use the correct helper? ;)
Experience would suggest not; users tend to use the simplest API that
gets their job done. And then wonder why they don't get the nice
performance numbers they were "promised". And, well, I tend to agree
that it's not terribly friendly to just go "use this other more
complicated API if you want proper performance". If we really mean that,
then we should formally deprecate xdp_redirect() as an API, IMO :)
-Toke
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Oliver Hartkopp @ 2019-02-26 11:04 UTC (permalink / raw)
To: Julius Niedworok, linux-wireless
Cc: ga58taw, david, nc, David S. Miller, Johannes Berg, Edward Cree,
Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
linux-kernel
In-Reply-To: <20190226094104.35192-1-julius.n@gmx.net>
Hi Julius,
On 26.02.19 10:40, Julius Niedworok wrote:
> In order to force delivery of TX status frames for research and debugging
> purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag
> is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled
> in all packets sent from that interface. IFF_ECHO can be set via
> /sys/class/net/<dev>/flags. The default is disabled.
(..)
> + /*
> + * Force TX status frames on ieee80211 devices.
> + * Since IFF_ECHO is used by CAN devices for a different
> + * purpose, we must check dev->ieee80211_ptr.
> + */
The reason for IFF_ECHO was, that the data frame which is sent onto the
wire (by one application) is not visible to all the other applications
on the same (local) host. Therefore a successful transmission on the
wire triggers the 'echo' of the sent content into the local host.
So what are you getting back after you enabled IFF_ECHO on your mac80211
device?
Is it just a 'status' about a sent packet, or is it the packet ('full
content') itself?
Regards,
Oliver
^ permalink raw reply
* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Martin Blumenstingl @ 2019-02-26 11:37 UTC (permalink / raw)
To: Anand Moon; +Cc: Bartosz Golaszewski, linux-amlogic, netdev
In-Reply-To: <CANAwSgSw4dCQnHv0cMSxDs7S=83NUh5B6M8m_-ooFQKv02=v_g@mail.gmail.com>
Hi Anand,
On Tue, Feb 26, 2019 at 11:26 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Martin,
>
> On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > hi Martin,
> >
> > +Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > I have seen Anand's your question in [0]:
> > > > only issue is I have is the each time their is random MAC address so I
> > > > get new IP from dhcp server.
> > > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> > >
> > > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > > pass it (via the .dtb) to the kernel.
> > > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > > for and example.
> > >
> > > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > > controllers: [2] (please note that the function implementing this
> > > binding was recently renamed: [3])
> > > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > > based binding yet
> > >
> > > Anand, if you want to work on this: feel free to do so!
> > > I have the SDHC MMC driver and a discussion about the power-domain
> > > drivers on my TODO-list, so I'm pretty busy at the moment.
> > >
> > >
> > > Regards
> > > Martin
> > >
> >
> > Thanks for your inputs :) 8)
> >
> > After enable CONFIG_MESON_MX_EFUSE and added the alias
> >
>
> As far as I can tell this nvmem consist of field.
> Board ID
> MAC address
> Serial Number
> UID
>
> I feel efues value in nvmem are read with following offset just for testing.
> but it also need some driver changes to read from secure memory
> by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> > On Odroid C1+
> > # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> > 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> > 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> > 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> > *
> > 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000 *00:1e:06:10:dc:ad*
> > mac address from nvmem
> > 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> > 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> > 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> > 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> > 0000200
> >
> arch/arm/boot/dts/meson8b.dtsi
> @@ -360,6 +360,18 @@
> compatible = "amlogic,meson8b-efuse";
> clocks = <&clkc CLKID_EFUSE>;
> clock-names = "core";
> +
> + board_sn: board-sn@0000000 {
> + reg = <0x0000000 0x10>;
> + };
> +
> + eth_mac: eth-mac@00001b4 {
> + reg = <0x00001b4 0x6>;
> + };
> +
> + board_sno: board-sno@00001c0 {
> + reg = <0x00001c0 0x30>;
> + };
>
> This is what I am looking into.
> If you have some input please share.
your findings are looking good to me!
I believe that some of the offsets are specific to the Odroid-C1.
this is what the Endless Mini EC-100's kernel uses: [0] (eth_mac
matches, but it seems that the other two don't)
Regards
Martin
[0] https://github.com/endlessm/linux-meson/blob/9969d20dc0f034e7a5addd4f6d28a8193e956b16/drivers/amlogic/efuse/efuse_version.c#L519
^ permalink raw reply
* [PATCH net 4/4] tls: Fix tls_device receive
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
Currently, the receive function fails to handle records already
decrypted by the device due to the commit mentioned below.
This commit advances the TLS record sequence number and prepares the context
to handle the next record.
Fixes: fedf201e1296 ("net: tls: Refactor control message handling on recv")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
net/tls/tls_sw.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index f515cd7e984e..85da10182d8d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1481,18 +1481,17 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
return err;
}
-
- rxm->full_len -= padding_length(ctx, tls_ctx, skb);
-
- rxm->offset += prot->prepend_size;
- rxm->full_len -= prot->overhead_size;
- tls_advance_record_sn(sk, &tls_ctx->rx, version);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
} else {
*zc = false;
}
+ rxm->full_len -= padding_length(ctx, tls_ctx, skb);
+ rxm->offset += prot->prepend_size;
+ rxm->full_len -= prot->overhead_size;
+ tls_advance_record_sn(sk, &tls_ctx->rx, version);
+ ctx->decrypted = true;
+ ctx->saved_data_ready(sk);
+
return err;
}
--
2.12.2
^ permalink raw reply related
* [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
application data TLS records. Moreover, it assumes that if the cipher
can be handled asynchronously, then all packets will be processed
asynchronously.
However, this assumption is not always true. Specifically, for AES-GCM
in TLS1.2, it causes data corruption, and breaks user applications.
This patch fixes this problem by separating the async capability from
the decryption operation result.
Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data records")
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Reviewed-by: Boris Pismenny <borisp@mellanox.com>
---
net/tls/tls_sw.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 4afa67b00aaf..f515cd7e984e 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
bool zc = false;
int to_decrypt;
int chunk = 0;
- bool async;
+ bool async_capable;
+ bool async = false;
skb = tls_wait_data(sk, psock, flags, timeo, &err);
if (!skb) {
@@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
/* Do not use async mode if record is non-data */
if (ctx->control == TLS_RECORD_TYPE_DATA)
- async = ctx->async_capable;
+ async_capable = ctx->async_capable;
else
- async = false;
+ async_capable = false;
err = decrypt_skb_update(sk, skb, &msg->msg_iter,
- &chunk, &zc, async);
+ &chunk, &zc, async_capable);
if (err < 0 && err != -EINPROGRESS) {
tls_err_abort(sk, EBADMSG);
goto recv_end;
}
- if (err == -EINPROGRESS)
+ if (err == -EINPROGRESS && async_capable) {
+ async = true;
num_async++;
- else if (prot->version == TLS_1_3_VERSION)
+ } else if (prot->version == TLS_1_3_VERSION) {
tlm->control = ctx->control;
+ }
/* If the type of records being processed is not known yet,
* set it to record type just dequeued. If it is already known,
--
2.12.2
^ permalink raw reply related
* [PATCH net 1/4] tls: Fix tls_device handling of partial records
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
Cleanup the handling of partial records while fixing a bug where the
tls_push_pending_closed_record function is using the software tls
context instead of the hardware context.
The bug resulted in the following crash:
[ 88.791229] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 88.793271] #PF error: [normal kernel read fault]
[ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD 22a156067 PMD 0
[ 88.795958] Oops: 0000 [#1] SMP PTI
[ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3
[ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls]
[ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd
4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213
[ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: 0000000000000000
[ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9af1e88a1980
[ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: ffff9af1ebeeb700
[ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: 0000000000000000
[ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) knlGS:0000000000000000
[ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: 00000000001406e0
[ 88.819328] Call Trace:
[ 88.820123] tls_push_data+0x628/0x6a0 [tls]
[ 88.821283] ? remove_wait_queue+0x20/0x60
[ 88.822383] ? n_tty_read+0x683/0x910
[ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls]
[ 88.824505] sock_sendmsg+0x36/0x50
[ 88.825492] sock_write_iter+0x87/0x100
[ 88.826521] __vfs_write+0x127/0x1b0
[ 88.827499] vfs_write+0xad/0x1b0
[ 88.828454] ksys_write+0x52/0xc0
[ 88.829378] do_syscall_64+0x5b/0x180
[ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 88.831603] RIP: 0033:0x7fcad1451680
[ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 1248.472564] #PF error: [normal kernel read fault]
[ 1248.473790] PGD 0 P4D 0
[ 1248.474642] Oops: 0000 [#1] SMP PTI
[ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0-rc4+ #3
[ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls]
[ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c
1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213
[ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: 0000000000000006
[ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: 0000000000000286
[ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: 00000000000002b1
[ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: 0000000000000000
[ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: 0000000000000000
[ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) knlGS:0000000000000000
[ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: 00000000001406e0
[ 1248.500136] Call Trace:
[ 1248.500998] ? tcp_check_oom+0xd0/0xd0
[ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls]
[ 1248.503411] inet_release+0x3c/0x60
[ 1248.504530] __sock_release+0x3d/0xb0
[ 1248.505611] sock_close+0x11/0x20
[ 1248.506612] __fput+0xb4/0x220
[ 1248.507559] task_work_run+0x88/0xa0
[ 1248.508617] do_exit+0x2cb/0xbc0
[ 1248.509597] ? core_sys_select+0x17a/0x280
[ 1248.510740] do_group_exit+0x39/0xb0
[ 1248.511789] get_signal+0x1d0/0x630
[ 1248.512823] do_signal+0x36/0x620
[ 1248.513822] exit_to_usermode_loop+0x5c/0xc6
[ 1248.515003] do_syscall_64+0x157/0x180
[ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1248.517456] RIP: 0033:0x7fb398bd3f53
[ 1248.518537] Code: Bad RIP value.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 20 ++++----------------
net/tls/tls_device.c | 9 +++++----
net/tls/tls_main.c | 13 -------------
3 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 9f4117ae2297..a528a082da73 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -199,10 +199,6 @@ struct tls_offload_context_tx {
(ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \
TLS_DRIVER_STATE_SIZE)
-enum {
- TLS_PENDING_CLOSED_RECORD
-};
-
struct cipher_context {
char *iv;
char *rec_seq;
@@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context *ctx,
int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
int flags);
-int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
- int flags, long *timeo);
-
static inline struct tls_msg *tls_msg(struct sk_buff *skb)
{
return (struct tls_msg *)strp_msg(skb);
}
-static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
+static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
{
- return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
+ return !!ctx->partially_sent_record;
}
static inline int tls_complete_pending_work(struct sock *sk,
@@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct sock *sk,
if (unlikely(sk->sk_write_pending))
rc = wait_on_pending_writer(sk, timeo);
- if (!rc && tls_is_pending_closed_record(ctx))
- rc = tls_push_pending_closed_record(sk, ctx, flags, timeo);
+ if (!rc && tls_is_partially_sent_record(ctx))
+ rc = tls_push_partial_record(sk, ctx, flags);
return rc;
}
-static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
-{
- return !!ctx->partially_sent_record;
-}
-
static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
{
return tls_ctx->pending_open_record_frags;
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index a5c17c47d08a..3e5e8e021a87 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk,
list_add_tail(&record->list, &offload_ctx->records_list);
spin_unlock_irq(&offload_ctx->lock);
offload_ctx->open_record = NULL;
- set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version);
for (i = 0; i < record->num_frags; i++) {
@@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk,
return -sk->sk_err;
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
- rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
- if (rc < 0)
- return rc;
+ if (tls_is_partially_sent_record(tls_ctx)) {
+ rc = tls_push_partial_record(sk, tls_ctx, flags);
+ if (rc < 0)
+ return rc;
+ }
pfrag = sk_page_frag(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index caff15b2f9b2..7e05af75536d 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
return tls_push_sg(sk, ctx, sg, offset, flags);
}
-int tls_push_pending_closed_record(struct sock *sk,
- struct tls_context *tls_ctx,
- int flags, long *timeo)
-{
- struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
-
- if (tls_is_partially_sent_record(tls_ctx) ||
- !list_empty(&ctx->tx_list))
- return tls_tx_records(sk, flags);
- else
- return tls_ctx->push_pending_record(sk, flags);
-}
-
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH net 2/4] tls: Fix write space handling
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
TLS device cannot use the sw context. This patch returns the original
tls device write space handler and moves the sw/device specific portions
to the relevant files.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 3 +++
net/tls/tls_device.c | 16 ++++++++++++++++
net/tls/tls_main.c | 17 +++++++++--------
net/tls/tls_sw.c | 15 +++++++++++++++
4 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index a528a082da73..9d7c53737b13 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock *sk)
return !!tls_sw_ctx_tx(ctx);
}
+int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
+int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
+
static inline struct tls_offload_context_rx *
tls_offload_ctx_rx(const struct tls_context *tls_ctx)
{
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 3e5e8e021a87..e8988b3f3236 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct sock *sk, int flags)
return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
}
+int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ int rc = 0;
+
+ if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
+ gfp_t sk_allocation = sk->sk_allocation;
+
+ sk->sk_allocation = GFP_ATOMIC;
+ rc = tls_push_partial_record(sk, ctx,
+ MSG_DONTWAIT | MSG_NOSIGNAL);
+ sk->sk_allocation = sk_allocation;
+ }
+
+ return rc;
+}
+
void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 7e05af75536d..11c1980a75cb 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
- struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
+ int rc;
/* If in_tcp_sendpages call lower protocol write space handler
* to ensure we wake up any waiting operations there. For example
@@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
return;
}
- /* Schedule the transmission if tx list is ready */
- if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
- /* Schedule the transmission */
- if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask))
- schedule_delayed_work(&tx_ctx->tx_work.work, 0);
- }
+#ifdef CONFIG_TLS_DEVICE
+ if (ctx->tx_conf == TLS_HW)
+ rc = tls_device_write_space(sk, ctx);
+ else
+#endif
+ rc = tls_sw_write_space(sk, ctx);
- ctx->sk_write_space(sk);
+ if (!rc)
+ ctx->sk_write_space(sk);
}
static void tls_ctx_free(struct tls_context *ctx)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 1cc830582fa8..4afa67b00aaf 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct *work)
release_sock(sk);
}
+int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
+
+ /* Schedule the transmission if tx list is ready */
+ if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
+ /* Schedule the transmission */
+ if (!test_and_set_bit(BIT_TX_SCHEDULED,
+ &tx_ctx->tx_bitmask))
+ schedule_delayed_work(&tx_ctx->tx_work.work, 0);
+ }
+
+ return 0;
+}
+
int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH net 0/4] tls: Fix issues in tls_device
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
This series fixes issues encountered in tls_device code paths,
which were introduced recently.
Additionally, this series includes a fix for tls software only receive flow,
which causes corruption of payload received by user space applications.
This series was tested using the OpenSSL integration of KTLS -
https://github.com/mellanox/openssl/tree/tls_rx3
Boris Pismenny (3):
tls: Fix tls_device handling of partial records
tls: Fix write space handling
tls: Fix tls_device receive
Eran Ben Elisha (1):
tls: Fix mixing between async capable and async
include/net/tls.h | 23 +++++++----------------
net/tls/tls_device.c | 25 +++++++++++++++++++++----
net/tls/tls_main.c | 30 +++++++++---------------------
net/tls/tls_sw.c | 45 +++++++++++++++++++++++++++++++--------------
4 files changed, 68 insertions(+), 55 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Anand Moon @ 2019-02-26 12:13 UTC (permalink / raw)
To: Martin Blumenstingl; +Cc: Bartosz Golaszewski, linux-amlogic, netdev
In-Reply-To: <CAFBinCCCnCBO4dbvGjDuiH6Ba-9mcg3Ax3-vkLipGijubNBFBA@mail.gmail.com>
Hi Martin,
On Tue, 26 Feb 2019 at 17:07, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Tue, Feb 26, 2019 at 11:26 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Martin,
> >
> > On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
> > >
> > > hi Martin,
> > >
> > > +Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> > > <martin.blumenstingl@googlemail.com> wrote:
> > > >
> > > > I have seen Anand's your question in [0]:
> > > > > only issue is I have is the each time their is random MAC address so I
> > > > > get new IP from dhcp server.
> > > > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> > > >
> > > > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > > > pass it (via the .dtb) to the kernel.
> > > > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > > > for and example.
> > > >
> > > > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > > > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > > > controllers: [2] (please note that the function implementing this
> > > > binding was recently renamed: [3])
> > > > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > > > based binding yet
> > > >
> > > > Anand, if you want to work on this: feel free to do so!
> > > > I have the SDHC MMC driver and a discussion about the power-domain
> > > > drivers on my TODO-list, so I'm pretty busy at the moment.
> > > >
> > > >
> > > > Regards
> > > > Martin
> > > >
> > >
> > > Thanks for your inputs :) 8)
> > >
> > > After enable CONFIG_MESON_MX_EFUSE and added the alias
> > >
> >
> > As far as I can tell this nvmem consist of field.
> > Board ID
> > MAC address
> > Serial Number
> > UID
> >
> > I feel efues value in nvmem are read with following offset just for testing.
> > but it also need some driver changes to read from secure memory
> > by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> > > On Odroid C1+
> > > # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> > > 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> > > 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> > > 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> > > *
> > > 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000 *00:1e:06:10:dc:ad*
> > > mac address from nvmem
> > > 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> > > 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> > > 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> > > 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> > > 0000200
> > >
> > arch/arm/boot/dts/meson8b.dtsi
> > @@ -360,6 +360,18 @@
> > compatible = "amlogic,meson8b-efuse";
> > clocks = <&clkc CLKID_EFUSE>;
> > clock-names = "core";
> > +
> > + board_sn: board-sn@0000000 {
> > + reg = <0x0000000 0x10>;
> > + };
> > +
> > + eth_mac: eth-mac@00001b4 {
> > + reg = <0x00001b4 0x6>;
> > + };
> > +
> > + board_sno: board-sno@00001c0 {
> > + reg = <0x00001c0 0x30>;
> > + };
> >
> > This is what I am looking into.
> > If you have some input please share.
> your findings are looking good to me!
>
> I believe that some of the offsets are specific to the Odroid-C1.
> this is what the Endless Mini EC-100's kernel uses: [0] (eth_mac
> matches, but it seems that the other two don't)
>
>
.title = "mac", //for the main network interface
.id = EFUSE_MAC_ID,
.offset = 436,
.enc_len = 6,
.data_len = 6,
offset for mac which is in dec *436* is equal to hex *0x1b4* which I
am using for testing now.
I am changing this offset setting so that I get consistence result out of it.
I will also check with armbina image and see if I can find some thing usefull.
Thanks for your valuable inputs once again :)
Best Regards
-Anand
>
> [0] https://github.com/endlessm/linux-meson/blob/9969d20dc0f034e7a5addd4f6d28a8193e956b16/drivers/amlogic/efuse/efuse_version.c#L519
^ permalink raw reply
* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Neil Horman @ 2019-02-26 12:29 UTC (permalink / raw)
To: Xin Long; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <CADvbK_dtfZnKQvWO5CMaVfaUg+yZUGH3TVN2xkWrqz7L+Z_MGQ@mail.gmail.com>
On Tue, Feb 26, 2019 at 12:15:54AM +0800, Xin Long wrote:
> On Mon, Feb 25, 2019 at 10:08 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Mon, Feb 25, 2019 at 09:20:44PM +0800, Xin Long wrote:
> > > On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > > > > sctp_hdr(skb) only works when skb->transport_header is set properly.
> > > > >
> > > > > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> > > > >
> > > > > skb->transport_header is not guaranteed to be right value for sctp.
> > > > > It will cause to fail to check the checksum for sctp packets.
> > > > >
> > > > > So fix it by using offset, which is always right in all places.
> > > > >
> > > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > ---
> > > > > include/net/sctp/checksum.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > > > > index 32ee65a..1c6e6c0 100644
> > > > > --- a/include/net/sctp/checksum.h
> > > > > +++ b/include/net/sctp/checksum.h
> > > > > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> > > > > static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> > > > > unsigned int offset)
> > > > > {
> > > > > - struct sctphdr *sh = sctp_hdr(skb);
> > > > > + struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> > > > > const struct skb_checksum_ops ops = {
> > > > > .update = sctp_csum_update,
> > > > > .combine = sctp_csum_combine,
> > > > > --
> > > > > 2.1.0
> > > > >
> > > > >
> > > > Shouldn't you use skb_set_transport_header and skb_transport_header here?
> > > you mean:
> > > skb_set_transport_header(skb, offset);
> > > sh = sctp_hdr(skb);
> > > ?
> > >
> > > There's no place counting on here to set transport_header.
> > > It will be a kinda redundant job, yet skb is 'const'.
> > >
> > I'm not sure what you mean by "theres no place counting here". We have the
> > transport header offset, and you're doing the exact same computation that that
> > function does. It seems like we should use it in case the underlying
> > implementation changes.
> 1. skb_set_transport_header() and sctp_hdr() are like:
> skb->transport_header = skb->data - skb->head;
> skb->transport_header += offset
> sh = skb->head + skb->transport_header;
>
> 2. in this patch:
> sh = (struct sctphdr *)(skb->data + offset); only
>
> I think the 2nd one is better.
>
> I feel it's weird to set transport_header here if it's only for
> sctp_hdr(skb) in here.
>
> As for "underlying implementation changes", I don't know exactly the case
> but there are quite a few places doing things like:
> *hdr = (struct *hdr *)(skb->data + hdroff);
>
> I'd think it's safe. no?
>
Safe, yes, it just doesn't seem right. I know you've pointed out several places
below that rapidly compute transport offsets in a one-off fashion, but at this
same time, the other primary transports (tcp and udp), all seems to use the
transport header to do their work (linearizing as necessecary, which sctp also
does in sctp_rcv, at least in most cases).
> >
> > I understand what you are saying regarding the use of a const variable there,
> > but perhaps thats an argument for removing the const storage classifier. Better
> > still, it would be good to figure out why all paths to this function don't
> > already set the transport header offset to begin with (addressing your redundant
> > comment)
> The issue was reported when going to nf_conntrack by br_netfilter's
> bridge-nf-call-iptables.
> As you can see on nf_conntrack_in() path, even iphdr is got by:
> iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
> It's impossible to set skb->transport_header when we're not sure iphdr
> in linearized memory.
>
But if the skb isn't linearized, computing the transport header manually isn't
going to help you anyway. You can see that in skb_header_pointer. If the
offset they are trying to get to is outside the bounds of the length of the skb
(i.e. the fragmented case), it calls skb_copy_bits to linearize the needed
segment. It seems we should be doing something simmilar. In most cases we are
already linearized from sctp_rcv (possibly all, I need to think about that). All
I'm really saying is that by using the skb apis we insulate ourselves from
potential changes in how skbs might work in the future. I'm not strictly bound
to setting the transport header, but we should definately be getting the
transport header via the skb utility functions wherever possible.
Neil
^ permalink raw reply
* RE: [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Vakul Garg @ 2019-02-26 12:38 UTC (permalink / raw)
To: Boris Pismenny, aviadye@mellanox.com, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: eranbe@mellanox.com
In-Reply-To: <20190226121235.20784-4-borisp@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Tuesday, February 26, 2019 5:43 PM
> To: aviadye@mellanox.com; davejwatson@fb.com;
> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
> <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: eranbe@mellanox.com; borisp@mellanox.com
> Subject: [PATCH net 3/4] tls: Fix mixing between async capable and async
>
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
> application data TLS records. Moreover, it assumes that if the cipher can be
> handled asynchronously, then all packets will be processed asynchronously.
>
> However, this assumption is not always true.
Could you please elaborate, what happens?
> Specifically, for AES-GCM in
> TLS1.2, it causes data corruption, and breaks user applications.
>
> This patch fixes this problem by separating the async capability from the
> decryption operation result.
>
> Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data
> records")
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Reviewed-by: Boris Pismenny <borisp@mellanox.com>
> ---
> net/tls/tls_sw.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> 4afa67b00aaf..f515cd7e984e 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
> bool zc = false;
> int to_decrypt;
> int chunk = 0;
> - bool async;
> + bool async_capable;
> + bool async = false;
>
> skb = tls_wait_data(sk, psock, flags, timeo, &err);
> if (!skb) {
> @@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
>
> /* Do not use async mode if record is non-data */
> if (ctx->control == TLS_RECORD_TYPE_DATA)
> - async = ctx->async_capable;
> + async_capable = ctx->async_capable;
> else
> - async = false;
> + async_capable = false;
>
> err = decrypt_skb_update(sk, skb, &msg->msg_iter,
> - &chunk, &zc, async);
> + &chunk, &zc, async_capable);
> if (err < 0 && err != -EINPROGRESS) {
> tls_err_abort(sk, EBADMSG);
> goto recv_end;
> }
>
> - if (err == -EINPROGRESS)
> + if (err == -EINPROGRESS && async_capable) {
Why do we need to check 'async_capable'?
Do we get err == -EINPROGRESS even when async_capable is false?
> + async = true;
> num_async++;
> - else if (prot->version == TLS_1_3_VERSION)
> + } else if (prot->version == TLS_1_3_VERSION) {
> tlm->control = ctx->control;
> + }
>
> /* If the type of records being processed is not known yet,
> * set it to record type just dequeued. If it is already known,
> --
> 2.12.2
^ permalink raw reply
* RE: [PATCH net 2/4] tls: Fix write space handling
From: Vakul Garg @ 2019-02-26 12:49 UTC (permalink / raw)
To: Boris Pismenny, aviadye@mellanox.com, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: eranbe@mellanox.com
In-Reply-To: <20190226121235.20784-3-borisp@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Tuesday, February 26, 2019 5:43 PM
> To: aviadye@mellanox.com; davejwatson@fb.com;
> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
> <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: eranbe@mellanox.com; borisp@mellanox.com
> Subject: [PATCH net 2/4] tls: Fix write space handling
>
> TLS device cannot use the sw context. This patch returns the original
> tls device write space handler and moves the sw/device specific portions
> to the relevant files.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records
> for performance")
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> ---
> include/net/tls.h | 3 +++
> net/tls/tls_device.c | 16 ++++++++++++++++
> net/tls/tls_main.c | 17 +++++++++--------
> net/tls/tls_sw.c | 15 +++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index a528a082da73..9d7c53737b13 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock
> *sk)
> return !!tls_sw_ctx_tx(ctx);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
> +
> static inline struct tls_offload_context_rx *
> tls_offload_ctx_rx(const struct tls_context *tls_ctx)
> {
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index 3e5e8e021a87..e8988b3f3236 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct
> sock *sk, int flags)
> return tls_push_data(sk, &msg_iter, 0, flags,
> TLS_RECORD_TYPE_DATA);
> }
>
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + int rc = 0;
> +
> + if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
> + gfp_t sk_allocation = sk->sk_allocation;
> +
> + sk->sk_allocation = GFP_ATOMIC;
> + rc = tls_push_partial_record(sk, ctx,
> + MSG_DONTWAIT |
> MSG_NOSIGNAL);
> + sk->sk_allocation = sk_allocation;
> + }
> +
> + return rc;
> +}
> +
> void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 7e05af75536d..11c1980a75cb 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct
> tls_context *ctx,
> static void tls_write_space(struct sock *sk)
> {
> struct tls_context *ctx = tls_get_ctx(sk);
> - struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> + int rc;
>
> /* If in_tcp_sendpages call lower protocol write space handler
> * to ensure we wake up any waiting operations there. For example
> @@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
> return;
> }
>
> - /* Schedule the transmission if tx list is ready */
> - if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> - /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx-
> >tx_bitmask))
> - schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> - }
> +#ifdef CONFIG_TLS_DEVICE
> + if (ctx->tx_conf == TLS_HW)
> + rc = tls_device_write_space(sk, ctx);
> + else
> +#endif
> + rc = tls_sw_write_space(sk, ctx);
>
> - ctx->sk_write_space(sk);
> + if (!rc)
Why do we need to check 'rc'?
If it is required, then ' ctx->sk_write_space(sk)' can move to tls_device_write_space()
since tls_sw_write_space() always returns '0'.
> + ctx->sk_write_space(sk);
> }
>
> static void tls_ctx_free(struct tls_context *ctx)
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 1cc830582fa8..4afa67b00aaf 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct
> *work)
> release_sock(sk);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> +
> + /* Schedule the transmission if tx list is ready */
> + if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> + /* Schedule the transmission */
> + if (!test_and_set_bit(BIT_TX_SCHEDULED,
> + &tx_ctx->tx_bitmask))
> + schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> + }
> +
> + return 0;
> +}
> +
> int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> --
> 2.12.2
^ 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