* Re: [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: Save switch rules
From: Florian Fainelli @ 2019-01-25 18:37 UTC (permalink / raw)
To: Miquel Raynal, Andrew Lunn, Vivien Didelot, David S. Miller
Cc: netdev, linux-kernel, Thomas Petazzoni, Gregory Clement,
Antoine Tenart, Maxime Chevallier, Nadav Haklai
In-Reply-To: <20190125095507.29334-2-miquel.raynal@bootlin.com>
Hi Miquel,
On 1/25/19 1:55 AM, Miquel Raynal wrote:
> The user might apply a specific switch configuration, with specific
> forwarding rules, VLAN, bridges, etc.
>
> During suspend to RAM the switch power will be turned off and the
> switch will lost its configuration. In an attempt to bring S2RAM
> support to the mv88e6xxx DSA, let's first save these rules in a
> per-chip list thanks to the mv88e6xxx_add/del_xxx_rule()
> helpers. These helpers are then called from various callbacks:
> * mv88e6xxx_port_fdb_add/del()
> * mv88e6xxx_port_mdb_add/del()
> * mv88e6xxx_port_vlan_add/del()
> * mv88e6xxx_port_bridge_join/leave()
> * mv88e6xxx_crosschip_bridge_join/leave()
>
> To avoid recursion problems when replaying the rules, the content of
> the above *_add()/*_join() callbacks has been moved in separate
> helpers with a '_' prefix. Hence, each callback just calls the
> corresponding helper and the corresponding *_add_xxx_rule().
None of this should be done in the driver IMHO, because this is
presumably applicable to all switch devices that lose their state during
suspend/resume, so at best, this should be moved to the core DSA layer,
but doing this means that we should also have a well established
contract between the DSA layer and individual switch drivers as far as
quiescing/saving/restoring state goes.
By moving things to the core we can also more tightly control what data
structures get used to represent e.g.: VLANs, FDBs, MDBs etc and
possibly push/utilize caching into the original subsystem. For instance
VLAN/bridge already do maintain caches of VLANs, so if we could somehow
expose those, we would not bloat the kernel's memory footprint by having
an additional layer to maintain with identical information.
Just my 2 cents.
--
Florian
^ permalink raw reply
* Re: [PATCH] r8169: Load MAC address from device tree if present
From: Heiner Kallweit @ 2019-01-25 18:34 UTC (permalink / raw)
To: Thierry Reding, David S. Miller
Cc: Realtek linux nic maintainers, netdev, devicetree, linux-kernel
In-Reply-To: <20190125101814.6262-1-thierry.reding@gmail.com>
On 25.01.2019 11:18, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> If the system was booted using a device tree and if the device tree
> contains a MAC address, use it instead of reading one from the EEPROM.
> This is useful in situations where the EEPROM isn't properly programmed
> or where the firmware wants to override the existing MAC address.
>
I rarely see DT-configured boards with RTL8168 network. Do you add this
patch because of a specific board?
And you state "if EEPROM isn't properly programmed": Did you come across
such a case?
In general the patch is fine with me, I just want to understand the
motivation. One further comment see inline.
As of today we already have the option to set a MAC from userspace
via ethtool.
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Based on net-next.
>
> drivers/net/ethernet/realtek/r8169.c | 35 +++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index f574b6b557f9..fd9edd643ca5 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -6957,6 +6957,21 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
> return pci_alloc_irq_vectors(tp->pci_dev, 1, 1, flags);
> }
>
[...]
> @@ -7252,20 +7268,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> u64_stats_init(&tp->rx_stats.syncp);
> u64_stats_init(&tp->tx_stats.syncp);
>
> - /* Get MAC address */
> - switch (tp->mac_version) {
> - u8 mac_addr[ETH_ALEN] __aligned(4);
> - case RTL_GIGA_MAC_VER_35 ... RTL_GIGA_MAC_VER_38:
> - case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_51:
> - *(u32 *)&mac_addr[0] = rtl_eri_read(tp, 0xe0, ERIAR_EXGMAC);
> - *(u16 *)&mac_addr[4] = rtl_eri_read(tp, 0xe4, ERIAR_EXGMAC);
> + /* get MAC address */
> + if (eth_platform_get_mac_address(&pdev->dev, mac_addr))
> + rtl_read_mac_address(tp, mac_addr);
> +
> + if (is_valid_ether_addr(mac_addr))
Here array mac_addr may be uninitialized (if platform defines no MAC
and chip version is not covered by the switch statement).
> + rtl_rar_set(tp, mac_addr);
>
> - if (is_valid_ether_addr(mac_addr))
> - rtl_rar_set(tp, mac_addr);
> - break;
> - default:
> - break;
> - }
> for (i = 0; i < ETH_ALEN; i++)
> dev->dev_addr[i] = RTL_R8(tp, MAC0 + i);
>
>
^ permalink raw reply
* Request for Quotation
From: Sasha Kelley @ 2019-01-24 19:19 UTC (permalink / raw)
To: netdev
Nice day to you!
My Names Sasha Kelley from Earthlink, Inc. Moscow Russia
There is an available invitation to tender suitable for your
products and I would like to inquire if your company will be
interested to submit offer for your products in Moscow Russia.
Please confirm interest by sending product catalog/price list
for
our review.
Looking forward to hearing from you.
Best Regards,
Area Manager
Sasha Kelley
Tel: +79017731031
E-mail: earthlink@zoho.com
Earthlink, Inc (Moscow).
^ permalink raw reply
* Re: [net 1/4] net/mlx5e: Force CHECKSUM_UNNECESSARY for short ethernet frames
From: Eric Dumazet @ 2019-01-25 18:31 UTC (permalink / raw)
To: Saeed Mahameed, xiyou.wangcong@gmail.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, Tariq Toukan,
edumazet@google.com, nikola.ciprich@linuxbox.cz
In-Reply-To: <478d619a2d677c91640294fbce403632c628050f.camel@mellanox.com>
On 01/25/2019 10:22 AM, Saeed Mahameed wrote:
> On Tue, 2019-01-22 at 13:30 -0800, Cong Wang wrote:
>> On Fri, Jan 18, 2019 at 4:25 PM Saeed Mahameed <saeedm@mellanox.com>
>> wrote:
>>> From: Cong Wang <xiyou.wangcong@gmail.com>
>>
>> I don't know why you want to make me as the author here, but I never
>> agree on _your_ updates on my previous patch.
>>
>> Please see below.
>>
>
> sorry, i just took your patch and worked on top of it, i thought you
> would like to get the credit for this.
>
I thought the issue was that the hardware csum provided by both mlx4 and mlx5 only
covered the bytes included in the IP (v4 or v6) frame.
Meaning that any non zero padding bytes are not checksummed.
If this can not be fixed by a firmware change, then the fix has nothing to do with a frame being
smaller than ETH_ZLEN + ETH_FCS_LEN
Alternative would be for the driver to trim the frame (pretend the skb->len is exactly the expected one),
but one could argue that tcpdump should be able to see padding bytes.
^ permalink raw reply
* Re: [PATCH] r8169: Load MAC address from device tree if present
From: Heiner Kallweit @ 2019-01-25 18:26 UTC (permalink / raw)
To: Andrew Lunn, Thierry Reding
Cc: David S. Miller, Realtek linux nic maintainers, netdev,
devicetree, linux-kernel
In-Reply-To: <20190125145711.GF12962@lunn.ch>
On 25.01.2019 15:57, Andrew Lunn wrote:
> On Fri, Jan 25, 2019 at 11:18:14AM +0100, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> If the system was booted using a device tree and if the device tree
>> contains a MAC address, use it instead of reading one from the EEPROM.
>> This is useful in situations where the EEPROM isn't properly programmed
>> or where the firmware wants to override the existing MAC address.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>> Based on net-next.
>>
>> drivers/net/ethernet/realtek/r8169.c | 35 +++++++++++++++++-----------
>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>> index f574b6b557f9..fd9edd643ca5 100644
>> --- a/drivers/net/ethernet/realtek/r8169.c
>> +++ b/drivers/net/ethernet/realtek/r8169.c
>> @@ -6957,6 +6957,21 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
>> return pci_alloc_irq_vectors(tp->pci_dev, 1, 1, flags);
>> }
>>
>> +static void rtl_read_mac_address(struct rtl8169_private *tp,
>> + u8 mac_addr[ETH_ALEN])
>> +{
>> + /* Get MAC address */
>> + switch (tp->mac_version) {
>> + case RTL_GIGA_MAC_VER_35 ... RTL_GIGA_MAC_VER_38:
>> + case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_51:
>> + *(u32 *)&mac_addr[0] = rtl_eri_read(tp, 0xe0, ERIAR_EXGMAC);
>> + *(u16 *)&mac_addr[4] = rtl_eri_read(tp, 0xe4, ERIAR_EXGMAC);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> DECLARE_RTL_COND(rtl_link_list_ready_cond)
>> {
>> return RTL_R8(tp, MCU) & LINK_LIST_RDY;
>> @@ -7148,6 +7163,7 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp)
>> static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>> {
>> const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
>> + u8 mac_addr[ETH_ALEN] __aligned(4);
>
> Hi Thierry
>
> Maybe now would be a good time to cleanup this __aligned(4), pointer
> aliasing, etc?
>
Andrew, for my understanding: What do you think is wrong with the
alignment requirement? It was introduced because we do a 32 bit access
to the start address of the array and want to avoid an unaligned access.
>> + /* get MAC address */
>> + if (eth_platform_get_mac_address(&pdev->dev, mac_addr))
>> + rtl_read_mac_address(tp, mac_addr);
>> +
>
> Maybe that could be made more readable with:
>
> err = eth_platform_get_mac_address(&pdev->dev, mac_addr);
> if (err)
> rtl_read_mac_address(tp, mac_addr);
>
> Thanks
> Andrew
> .
>
^ permalink raw reply
* Re: WoL broken in r8169.c since kernel 4.19
From: Heiner Kallweit @ 2019-01-25 18:22 UTC (permalink / raw)
To: Marc Haber; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190125120210.GC27062@torres.zugschlus.de>
On 25.01.2019 13:02, Marc Haber wrote:
> On Fri, Jan 25, 2019 at 07:49:56AM +0100, Heiner Kallweit wrote:
>> thanks a lot for the detailed analysis. That this ethtool sequence
>>
>> ethtool -s <if> wol d
>> ethtool -s <if> wol g
>>
>> helps makes me think that the following patch should help too.
>> Could you please test?
>
> That patch didn't apply cleanly because the rtl_init_one in kernel
> 4.20.4 is missing the INIT_WORK call at this place.
>
> And it doesn't change the behavior, the two ethtool calls are needed so
> that the host wakes up from suspend to ram on a magic packet.
>
Then I'm slowly running out of ideas. New in 4.19 is a check for invalid
WoL flags, but usually the caller should warn if -EINVAL is returned.
Nevertheless, could you try the following and check whether the warning
is triggered?
---
drivers/net/ethernet/realtek/r8169.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 04c403539..a27cfc6e7 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1491,8 +1491,10 @@ static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
struct rtl8169_private *tp = netdev_priv(dev);
struct device *d = tp_to_dev(tp);
- if (wol->wolopts & ~WAKE_ANY)
+ if (wol->wolopts & ~WAKE_ANY) {
+ pr_warn("invalid WoL value: 0x%08x\n", wol->wolopts);
return -EINVAL;
+ }
pm_runtime_get_noresume(d);
--
2.20.1
> Greetings
> Marc
>
Heiner
^ permalink raw reply related
* Re: [net 1/4] net/mlx5e: Force CHECKSUM_UNNECESSARY for short ethernet frames
From: Saeed Mahameed @ 2019-01-25 18:22 UTC (permalink / raw)
To: xiyou.wangcong@gmail.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, Tariq Toukan,
edumazet@google.com, nikola.ciprich@linuxbox.cz
In-Reply-To: <CAM_iQpXjmJBrq8gnQ899JCZW_dFSuNmVQvbFQB0V=G1i=YSDrg@mail.gmail.com>
On Tue, 2019-01-22 at 13:30 -0800, Cong Wang wrote:
> On Fri, Jan 18, 2019 at 4:25 PM Saeed Mahameed <saeedm@mellanox.com>
> wrote:
> > From: Cong Wang <xiyou.wangcong@gmail.com>
>
> I don't know why you want to make me as the author here, but I never
> agree on _your_ updates on my previous patch.
>
> Please see below.
>
sorry, i just took your patch and worked on top of it, i thought you
would like to get the credit for this.
>
> > drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > index 1d0bb5ff8c26..f86e4804e83e 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> > @@ -732,6 +732,8 @@ static u8 get_ip_proto(struct sk_buff *skb, int
> > network_depth, __be16 proto)
> > ((struct ipv6hdr
> > *)ip_p)->nexthdr;
> > }
> >
> > +#define short_frame(size) ((size) <= ETH_ZLEN + ETH_FCS_LEN)
> > +
>
> I don't agree on unconditionally comparing with ETH_ZLEN +
> ETH_FCS_LEN.
>
This is more relaxed and it covers both cases unconditionally.
>
> > static inline void mlx5e_handle_csum(struct net_device *netdev,
> > struct mlx5_cqe64 *cqe,
> > struct mlx5e_rq *rq,
> > @@ -754,6 +756,17 @@ static inline void mlx5e_handle_csum(struct
> > net_device *netdev,
> > if (unlikely(test_bit(MLX5E_RQ_STATE_NO_CSUM_COMPLETE, &rq-
> > >state)))
> > goto csum_unnecessary;
> >
> > + /* CQE csum doesn't cover padding octets in short ethernet
> > + * frames. And the pad field is appended prior to
> > calculating
> > + * and appending the FCS field.
> > + *
> > + * Detecting these padded frames requires to verify and
> > parse
> > + * IP headers, so we simply force all those small frames to
> > be
> > + * CHECKSUM_UNNECESSARY even if they are not padded.
>
> This is inaccurate and misleading, it is unnecessary only if the
> packet
> passes the if check right below the goto label 'csum_unnecessary',
> otherwise still a CHECKSUM_NONE. IOW, you are not forcing anything
> here.
>
yes, the comment is not 100% accurate, but it delivers the message.
> > + */
> > + if (short_frame(skb->len))
>
> Missed an "unlikely()". Short frames are rare, comparing to non-short
> ones.
>
> I respect your judgement on CHECKSUM_UNNECESSARY, even when
> I still disagree with you. Please respect me by not forcing me to
> accept
> any updates from you, IOW, kindly removing my name from anything
> in this commit, SoB and authorship.
>
> Thank you for your understanding!
Again sorry about this, will be more careful in the future.
Thanks for your support and great work.
^ permalink raw reply
* Re: [PATCH mlx5-next 4/4] net/mlx5: Remove spinlock support from mlx5_write64
From: Saeed Mahameed @ 2019-01-25 18:16 UTC (permalink / raw)
To: Jason Gunthorpe, saeedm@dev.mellanox.co.il
Cc: Eran Ben Elisha, netdev@vger.kernel.org, Maxim Mikityanskiy,
Leon Romanovsky, linux-rdma@vger.kernel.org
In-Reply-To: <20190121182200.GI25149@mellanox.com>
On Mon, 2019-01-21 at 18:22 +0000, Jason Gunthorpe wrote:
> On Mon, Jan 21, 2019 at 10:12:58AM -0800, Saeed Mahameed wrote:
> > On Mon, Jan 21, 2019 at 8:46 AM Jason Gunthorpe <jgg@mellanox.com>
> > wrote:
> > > On Sat, Jan 19, 2019 at 12:43:14AM -0700, Leon Romanovsky wrote:
> > > > You need to do one of two things:
> > > > 1. Require CONFIG_64BIT and delete this 32bit code.
> > > > 2. Declare global mlx5 DB spinlock and use on 32bit systems,
> > > > something
> > > > like this:
> > > > #if BITS_PER_LONG == 64
> > > > __raw_writeq(*(u64 *)val, dest);
> > > > #else
> > > > spin_lock_irqsave(doorbell_lock, flags);
> > > > __raw_writel((__force u32) val[0], dest);
> > > > __raw_writel((__force u32) val[1], dest + 4);
> > > > spin_unlock_irqrestore(doorbell_lock, flags);
> > > > #endif
> > >
> > > And why is this code using the __raw_ versions? Seems wrong
> > > too...
> > >
> >
> > for 64 and 32 as well?
>
> yes
>
> > what is wrong with the raw version ?
>
> It should only be used by arch code (or in drivers linked to a
> specific arch). The actual properties of the 'raw' version are arch
> specific and make it hard to know if the driver will work on
> different
> archs. ie some arches may not byte swap their raw accessors, or may
> omit barriers.
>
> Most likely this just wants to be writeq for 64 bit and
> writel_relaxed() & writel() for 32 bit - unless there was some reason
> to have used __raw versions in the first place (in which case a
> comment is missing).
Ok, after some internal discussion it seems that
{read,write}{b,w,l,q}_relaxed() can be used instead of the __raw API
currently used in the driver, Adding as a future task.
This has nothing to do the the current cleanup patch.
Thanks,
Saeed.
>
> Jason
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Use helpers to access MMD PHY registers
From: Heiner Kallweit @ 2019-01-25 18:11 UTC (permalink / raw)
To: Andrew Lunn, Carlo Caione; +Cc: f.fainelli, davem, netdev, linux-kernel
In-Reply-To: <20190125150013.GG12962@lunn.ch>
On 25.01.2019 16:00, Andrew Lunn wrote:
> On Fri, Jan 25, 2019 at 12:35:10PM +0000, Carlo Caione wrote:
>> Libphy provides a standard set of helpers to access the MMD PHY
>> registers. Use those instead of relying on custom driver-specific
>> functions.
>
> Hi Carlo
>
> Maybe deja vu, but i thought a similar patch went by recently?
>
Andrew, maybe you mean this one?
9b420eff9fcf ("net: phy: micrel: use phy_read_mmd and phy_write_mmd")
> Anyway,
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
> Andrew
>
>>
>> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
>> ---
>> drivers/net/phy/at803x.c | 16 +++-------------
>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>> index f9432d053a22..23ba76f8d950 100644
>> --- a/drivers/net/phy/at803x.c
>> +++ b/drivers/net/phy/at803x.c
>> @@ -39,9 +39,6 @@
>> #define AT803X_LOC_MAC_ADDR_0_15_OFFSET 0x804C
>> #define AT803X_LOC_MAC_ADDR_16_31_OFFSET 0x804B
>> #define AT803X_LOC_MAC_ADDR_32_47_OFFSET 0x804A
>> -#define AT803X_MMD_ACCESS_CONTROL 0x0D
>> -#define AT803X_MMD_ACCESS_CONTROL_DATA 0x0E
>> -#define AT803X_FUNC_DATA 0x4003
>> #define AT803X_REG_CHIP_CONFIG 0x1f
>> #define AT803X_BT_BX_REG_SEL 0x8000
>>
>> @@ -168,16 +165,9 @@ static int at803x_set_wol(struct phy_device *phydev,
>> if (!is_valid_ether_addr(mac))
>> return -EINVAL;
>>
>> - for (i = 0; i < 3; i++) {
>> - phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
>> - AT803X_DEVICE_ADDR);
>> - phy_write(phydev, AT803X_MMD_ACCESS_CONTROL_DATA,
>> - offsets[i]);
>> - phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
>> - AT803X_FUNC_DATA);
>> - phy_write(phydev, AT803X_MMD_ACCESS_CONTROL_DATA,
>> - mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
>> - }
>> + for (i = 0; i < 3; i++)
>> + phy_write_mmd(phydev, AT803X_DEVICE_ADDR, offsets[i],
>> + mac[(i * 2) + 1] | (mac[(i * 2)] << 8));
>>
>> value = phy_read(phydev, AT803X_INTR_ENABLE);
>> value |= AT803X_INTR_ENABLE_WOL;
>> --
>> 2.19.1
>>
> .
>
^ permalink raw reply
* Re: [PATCH mlx5-next 0/4] mlx5 next misc updates
From: Saeed Mahameed @ 2019-01-25 18:08 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Jason Gunthorpe
In-Reply-To: <20190124123041.GS3604@mtr-leonro.mtl.com>
On Thu, Jan 24, 2019 at 4:30 AM Leon Romanovsky <leonro@mellanox.com> wrote:
>
> On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> > Hi all,
> >
> > This series includes updates to mlx5-next shared branch.
> >
> > 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> > 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
> > 3) from Michael Guralnik, Add pci AtomicOps request
> >
> > Thanks,
> > Saeed.
> >
> > ---
> >
> > Jason Gunthorpe (1):
> > net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> >
> > Michael Guralnik (1):
> > net/mlx5: Add pci AtomicOps request
>
> Those two were applied to mlx5-next branch.
>
> ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
> e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
>
> > Maxim Mikityanskiy (2):
> > net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> > net/mlx5: Remove spinlock support from mlx5_write64
>
> Those two needs extra work,
What extra work ?
>
> Thanks
>
> >
> > --
> > 2.20.1
> >
^ permalink raw reply
* Re: [PATCH iproute2-master] bridge: fdb: Fix FDB dump with strict checking disabled
From: Ivan Vecera @ 2019-01-25 18:00 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, stephen, dsahern, liali, mlxsw
In-Reply-To: <20190125170837.6434-1-idosch@mellanox.com>
----- Ido Schimmel <idosch@mellanox.com> wrote:
> While iproute2 correctly uses ifinfomsg struct as the ancillary header
> when requesting an FDB dump on old kernels, it sets the message type to
> RTM_GETLINK. This results in wrong reply being returned.
>
> Fix this by using RTM_GETNEIGH instead.
>
> Before:
> $ bridge fdb show brport dummy0
> Not RTM_NEWNEIGH: 00000158 00000010 00000002
>
> After:
> $ bridge fdb show brport dummy0
> 2a:0b:41:1c:92:d3 vlan 1 master br0 permanent
> 2a:0b:41:1c:92:d3 master br0 permanent
> 33:33:00:00:00:01 self permanent
> 01:00:5e:00:00:01 self permanent
>
> Fixes: 05880354c2cf ("bridge: fdb: Fix filtering with strict checking disabled")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: LiLiang <liali@redhat.com>
> ---
> bridge/fdb.c | 3 +--
> include/libnetlink.h | 3 +++
> lib/libnetlink.c | 23 +++++++++++++++++++++++
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index f75e953a80e2..c4bf40390f42 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -346,8 +346,7 @@ static int fdb_show(int argc, char **argv)
> if (rth.flags & RTNL_HANDLE_F_STRICT_CHK)
> rc = rtnl_neighdump_req(&rth, PF_BRIDGE, fdb_dump_filter);
> else
> - rc = rtnl_linkdump_req_filter_fn(&rth, PF_BRIDGE,
> - fdb_linkdump_filter);
> + rc = rtnl_fdb_linkdump_req_filter_fn(&rth, fdb_linkdump_filter);
> if (rc < 0) {
> perror("Cannot send dump request");
> exit(1);
> diff --git a/include/libnetlink.h b/include/libnetlink.h
> index 0854d6ad2fab..503b3ec11bb6 100644
> --- a/include/libnetlink.h
> +++ b/include/libnetlink.h
> @@ -81,6 +81,9 @@ int rtnl_linkdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
> int rtnl_linkdump_req_filter_fn(struct rtnl_handle *rth, int fam,
> req_filter_fn_t fn)
> __attribute__((warn_unused_result));
> +int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
> + req_filter_fn_t filter_fn)
> + __attribute__((warn_unused_result));
> int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
> __attribute__((warn_unused_result));
> int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req,
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index 110f47bcd65a..7b392862cb91 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -503,6 +503,29 @@ int rtnl_linkdump_req_filter_fn(struct rtnl_handle *rth, int family,
> return __rtnl_linkdump_req(rth, family);
> }
>
> +int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
> + req_filter_fn_t filter_fn)
> +{
> + struct {
> + struct nlmsghdr nlh;
> + struct ifinfomsg ifm;
> + char buf[128];
> + } req = {
> + .nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
> + .nlh.nlmsg_type = RTM_GETNEIGH,
> + .nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
> + .nlh.nlmsg_seq = rth->dump = ++rth->seq,
> + .ifm.ifi_family = PF_BRIDGE,
> + };
> + int err;
> +
> + err = filter_fn(&req.nlh, sizeof(req));
> + if (err)
> + return err;
> +
> + return send(rth->fd, &req, sizeof(req), 0);
> +}
> +
> int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
> {
> struct {
> --
> 2.20.1
>
Acked-by: Ivan Vecera <ivecera@redhat.com>
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/7] samples/bpf: Convert XDP samples to libbpf usage
From: Maciej Fijałkowski @ 2019-01-25 17:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: daniel, ast, netdev, jakub.kicinski
In-Reply-To: <20190125093027.28203d2f@redhat.com>
On Fri, 25 Jan 2019 09:30:27 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Thu, 24 Jan 2019 22:47:43 +0100
> Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
>
> > From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> >
> > Some of XDP samples that are attaching the bpf program to the interface
> > via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
> > loading and manipulating the ebpf program and maps. Convert them to do
> > this through libbpf usage and remove bpf_load from the picture.
> >
> > While at it remove what looks like debug leftover in
> > xdp_redirect_map_user.c
> >
> > In xdp_redirect_cpu, change the way that the program to be loaded onto
> > interface is chosen - user now needs to pass the program's section name
> > instead of the relative number. In case of typo print out the section
> > names to choose from.
> >
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > samples/bpf/Makefile | 8 +-
> > samples/bpf/xdp_redirect_cpu_user.c | 145
> > +++++++++++++++++++++++++----------- samples/bpf/xdp_redirect_map_user.c |
> > 47 ++++++++---- samples/bpf/xdp_redirect_user.c | 44 ++++++++---
> > samples/bpf/xdp_router_ipv4_user.c | 75 +++++++++++++------
> > samples/bpf/xdp_tx_iptunnel_user.c | 37 ++++++---
> > 6 files changed, 253 insertions(+), 103 deletions(-)
>
> Thanks a lot for converting sample/bpf programs to use libbpf. It have
> been on my todo list for a very long time.
>
> You didn't implement the matching of prog-names in a iproute2 like
> fashion, as I described, but the important part was converting this to
> libbpf, so I'll Ack this anyway.
>
I will post it as a follow-up patch once I'm done with this series and we got
this (hopefully) merged, if you don't mind.
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
Thank you!
> (I acked V1 just before... meant to ACK this V3)
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 5/7] samples/bpf: Add a "force" flag to XDP samples
From: Maciej Fijałkowski @ 2019-01-25 17:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: daniel, ast, netdev, jakub.kicinski,
Toke Høiland-Jørgensen
In-Reply-To: <20190125093822.735c6a27@redhat.com>
On Fri, 25 Jan 2019 09:38:22 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Thu, 24 Jan 2019 22:47:45 +0100
> Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
>
> > diff --git a/samples/bpf/xdp_sample_pkts_user.c
> > b/samples/bpf/xdp_sample_pkts_user.c index 5f5828ee0761..362ad35b524d 100644
> > --- a/samples/bpf/xdp_sample_pkts_user.c
> > +++ b/samples/bpf/xdp_sample_pkts_user.c
> > @@ -13,6 +13,8 @@
> > #include <libbpf.h>
> > #include <bpf/bpf.h>
> > #include <sys/resource.h>
> > +#include <libgen.h>
> > +#include <linux/if_link.h>
> >
> > #include "perf-sys.h"
> > #include "trace_helpers.h"
> > @@ -21,12 +23,13 @@
> > static int pmu_fds[MAX_CPUS], if_idx;
> > static struct perf_event_mmap_page *headers[MAX_CPUS];
> > static char *if_name;
> > +static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >
> > static int do_attach(int idx, int fd, const char *name)
> > {
> > int err;
> >
> > - err = bpf_set_link_xdp_fd(idx, fd, 0);
> > + err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
> > if (err < 0)
> > printf("ERROR: failed to attach program to %s\n", name);
> >
> > @@ -98,21 +101,42 @@ static void sig_handler(int signo)
> > exit(0);
> > }
> >
> > +static void usage(const char *prog)
> > +{
> > + fprintf(stderr,
> > + "%s: %s [OPTS] IFINDEX\n\n"
> ^^^^^^^
> The usage help text is wrong, this sample takes the interface name as input.
>
It can take ifindex as well as ifname actually as you're saying later in this
mail - so I'm going to fix this message to "(...) IFNAME/IFINDEX" in v4.
Hope that makes sense?
>
> > + "OPTS:\n"
> > + " -F force loading prog\n",
> > + __func__, prog);
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> > struct bpf_prog_load_attr prog_load_attr = {
> > .prog_type = BPF_PROG_TYPE_XDP,
> > };
> > + const char *optstr = "F";
> > + int prog_fd, map_fd, opt;
> > struct bpf_object *obj;
> > struct bpf_map *map;
> > - int prog_fd, map_fd;
> > char filename[256];
> > int ret, err, i;
> > int numcpus;
> >
> > - if (argc < 2) {
> > - printf("Usage: %s <ifname>\n", argv[0]);
> ^^^^^^^^
> Old help text says <ifname>
>
> > + while ((opt = getopt(argc, argv, optstr)) != -1) {
> > + switch (opt) {
> > + case 'F':
> > + xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
> > + break;
> > + default:
> > + usage(basename(argv[0]));
> > + return 1;
> > + }
> > + }
> > +
> > + if (optind == argc) {
> > + usage(basename(argv[0]));
> > return 1;
> > }
> >
> > @@ -143,16 +167,16 @@ int main(int argc, char **argv)
> > }
> > map_fd = bpf_map__fd(map);
> >
> > - if_idx = if_nametoindex(argv[1]);
> > + if_idx = if_nametoindex(argv[optind]);
>
> The we convert the ifname to ifindex.
>
> > if (!if_idx)
> > - if_idx = strtoul(argv[1], NULL, 0);
> > + if_idx = strtoul(argv[optind], NULL, 0);
> >
>
> We do support falling back to using this as an ifindex.
>
>
> > if (!if_idx) {
> > fprintf(stderr, "Invalid ifname\n");
> > return 1;
> > }
> > - if_name = argv[1];
> > - err = do_attach(if_idx, prog_fd, argv[1]);
> > + if_name = argv[optind];
> > + err = do_attach(if_idx, prog_fd, if_name);
> > if (err)
> > return err;
>
>
^ permalink raw reply
* Re: [PATCH iproute2-next 05/19] rdma: Move resource PD logic to separate file
From: David Ahern @ 2019-01-25 17:19 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20190122183125.15920-6-leon@kernel.org>
On 1/22/19 11:31 AM, Leon Romanovsky wrote:
> +static const
> +struct filters pd_valid_filters[MAX_NUMBER_OF_FILTERS] = {
> + { .name = "dev", .is_number = false },
> + { .name = "users", .is_number = true },
> + { .name = "pid", .is_number = true },
> + { .name = "ctxn", .is_number = true },
> + { .name = "pdn", .is_number = true },
> + { .name = "ctxn", .is_number = true }
> +};
These should not be in a header file. Looks like you have done this in
other patches as well.
^ permalink raw reply
* Re: [PATCH iproute2-master] bridge: fdb: Fix FDB dump with strict checking disabled
From: David Ahern @ 2019-01-25 17:12 UTC (permalink / raw)
To: Ido Schimmel, netdev@vger.kernel.org
Cc: stephen@networkplumber.org, ivecera@redhat.com, liali@redhat.com,
mlxsw
In-Reply-To: <20190125170837.6434-1-idosch@mellanox.com>
On 1/25/19 10:09 AM, Ido Schimmel wrote:
> While iproute2 correctly uses ifinfomsg struct as the ancillary header
> when requesting an FDB dump on old kernels, it sets the message type to
> RTM_GETLINK. This results in wrong reply being returned.
>
> Fix this by using RTM_GETNEIGH instead.
>
> Before:
> $ bridge fdb show brport dummy0
> Not RTM_NEWNEIGH: 00000158 00000010 00000002
>
> After:
> $ bridge fdb show brport dummy0
> 2a:0b:41:1c:92:d3 vlan 1 master br0 permanent
> 2a:0b:41:1c:92:d3 master br0 permanent
> 33:33:00:00:00:01 self permanent
> 01:00:5e:00:00:01 self permanent
>
> Fixes: 05880354c2cf ("bridge: fdb: Fix filtering with strict checking disabled")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: LiLiang <liali@redhat.com>
> ---
> bridge/fdb.c | 3 +--
> include/libnetlink.h | 3 +++
> lib/libnetlink.c | 23 +++++++++++++++++++++++
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [PATCH iproute2-master] bridge: fdb: Fix FDB dump with strict checking disabled
From: Ido Schimmel @ 2019-01-25 17:09 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: stephen@networkplumber.org, dsahern@gmail.com, ivecera@redhat.com,
liali@redhat.com, mlxsw, Ido Schimmel
While iproute2 correctly uses ifinfomsg struct as the ancillary header
when requesting an FDB dump on old kernels, it sets the message type to
RTM_GETLINK. This results in wrong reply being returned.
Fix this by using RTM_GETNEIGH instead.
Before:
$ bridge fdb show brport dummy0
Not RTM_NEWNEIGH: 00000158 00000010 00000002
After:
$ bridge fdb show brport dummy0
2a:0b:41:1c:92:d3 vlan 1 master br0 permanent
2a:0b:41:1c:92:d3 master br0 permanent
33:33:00:00:00:01 self permanent
01:00:5e:00:00:01 self permanent
Fixes: 05880354c2cf ("bridge: fdb: Fix filtering with strict checking disabled")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: LiLiang <liali@redhat.com>
---
bridge/fdb.c | 3 +--
include/libnetlink.h | 3 +++
lib/libnetlink.c | 23 +++++++++++++++++++++++
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index f75e953a80e2..c4bf40390f42 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -346,8 +346,7 @@ static int fdb_show(int argc, char **argv)
if (rth.flags & RTNL_HANDLE_F_STRICT_CHK)
rc = rtnl_neighdump_req(&rth, PF_BRIDGE, fdb_dump_filter);
else
- rc = rtnl_linkdump_req_filter_fn(&rth, PF_BRIDGE,
- fdb_linkdump_filter);
+ rc = rtnl_fdb_linkdump_req_filter_fn(&rth, fdb_linkdump_filter);
if (rc < 0) {
perror("Cannot send dump request");
exit(1);
diff --git a/include/libnetlink.h b/include/libnetlink.h
index 0854d6ad2fab..503b3ec11bb6 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -81,6 +81,9 @@ int rtnl_linkdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
int rtnl_linkdump_req_filter_fn(struct rtnl_handle *rth, int fam,
req_filter_fn_t fn)
__attribute__((warn_unused_result));
+int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
+ req_filter_fn_t filter_fn)
+ __attribute__((warn_unused_result));
int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
__attribute__((warn_unused_result));
int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req,
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 110f47bcd65a..7b392862cb91 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -503,6 +503,29 @@ int rtnl_linkdump_req_filter_fn(struct rtnl_handle *rth, int family,
return __rtnl_linkdump_req(rth, family);
}
+int rtnl_fdb_linkdump_req_filter_fn(struct rtnl_handle *rth,
+ req_filter_fn_t filter_fn)
+{
+ struct {
+ struct nlmsghdr nlh;
+ struct ifinfomsg ifm;
+ char buf[128];
+ } req = {
+ .nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+ .nlh.nlmsg_type = RTM_GETNEIGH,
+ .nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
+ .nlh.nlmsg_seq = rth->dump = ++rth->seq,
+ .ifm.ifi_family = PF_BRIDGE,
+ };
+ int err;
+
+ err = filter_fn(&req.nlh, sizeof(req));
+ if (err)
+ return err;
+
+ return send(rth->fd, &req, sizeof(req), 0);
+}
+
int rtnl_statsdump_req_filter(struct rtnl_handle *rth, int fam, __u32 filt_mask)
{
struct {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] tcp: allow zerocopy with fastopen
From: Eric Dumazet @ 2019-01-25 17:06 UTC (permalink / raw)
To: Willem de Bruijn, netdev
Cc: davem, ycheng, edumazet, alexey.kodanev, Willem de Bruijn
In-Reply-To: <20190125161723.75429-1-willemdebruijn.kernel@gmail.com>
On 01/25/2019 08:17 AM, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Accept MSG_ZEROCOPY in all the TCP states that allow sendmsg. Remove
> the explicit check for ESTABLISHED and CLOSE_WAIT states.
>
> This requires correctly handling zerocopy state (uarg, sk_zckey) in
> all paths reachable from other TCP states. Such as the EPIPE case
> in sk_stream_wait_connect, which a sendmsg() in incorrect state will
> now hit. Most paths are already safe.
>
> Only extension needed is for TCP Fastopen active open. This can build
> an skb with data in tcp_send_syn_data. Pass the uarg along with other
> fastopen state, so that this skb also generates a zerocopy
> notification on release.
>
> Tested with active and passive tcp fastopen packetdrill scripts at
> https://github.com/wdebruij/packetdrill/commit/1747eef03d25a2404e8132817d0f1244fd6f129d
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
Let see if syzkaller finds issues with this :)
Signed-off-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs
From: Russell King - ARM Linux admin @ 2019-01-25 16:48 UTC (permalink / raw)
To: John David Anglin; +Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, netdev
In-Reply-To: <c6e38a08-d76a-7fa9-5921-b632551ebcff@bell.net>
On Fri, Jan 25, 2019 at 11:30:54AM -0500, John David Anglin wrote:
> This also makes me wonder about this SD interrupt:
> 43: 0 0 GPIO1 3 Edge d00d0000.sdhci cd
> Many poeple have trouble with SD cards on reboot.
[off topic, but a relevant reply to the above]
Very likely unrelated to edge interrupts.
For a signal that goes low when a card is inserted and high when
removed, you really need an edge interrupt to avoid flooding the CPU
with a "stuck" interrupt - if you were to use an active low interrupt
for card detection, and the signal is pulled low by card insertion,
the interrupt remains active until you remove the card!
In any case, I've seen the same with some early SolidRun platforms
when using high speed modes.
Most issues with SD cards at reboot is where the card is left in low
voltage/high speed mode, the reboot happens, and then you try to talk
to it in low-speed mode (as required by the specs for card detection/
initialisation) and they don't respond. The only way out of that is
to reset the card - and the only way to reset the card is to remove
power from it.
If you're lucky enough to have a "regulator" which can be switched
off at reboot, that may not be sufficient - if there's a capacitor on
the SD card's supply line downstream from the regulator, the card can
be drawing soo little current that the voltage doesn't fall
sufficiently to cause the card to power down.
If you don't have any means to reset the card like that, then the only
workaround is to reduce the maximum speed - and avoid going to 1.8V
signalling mode.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: Kernel memory corruption in CIPSO labeled TCP packets processing.
From: Paul Moore @ 2019-01-25 16:45 UTC (permalink / raw)
To: Nazarov Sergey
Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
netdev@vger.kernel.org, Casey Schaufler
In-Reply-To: <11471341548341163@sas2-7b909973f402.qloud-c.yandex.net>
On Thu, Jan 24, 2019 at 9:46 AM Nazarov Sergey <s-nazarov@yandex.ru> wrote:
> 22.01.2019, 20:48, "Paul Moore" <paul@paul-moore.com>:
> >
> > Yes, exactly. If you don't pass the skb it shouldn't attempt to call
> > icmp_send() in case of error.
> >
> > --
> > paul moore
> > www.paul-moore.com
>
> You are right, sorry. We can do that without ip_options_compile modification.
> Simplified patch 2:
> ---
> net/ipv4/cipso_ipv4.c | 18 ++++++++++++++++--
> 1 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 777fa3b..a4ed0a4 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1735,13 +1735,27 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
> */
> void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
> {
> + unsigned char optbuf[sizeof(struct ip_options) + 40];
> + struct ip_options *opt = (struct ip_options *)optbuf;
> +
> if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
> return;
>
> + /*
> + * We might be called above the IP layer,
> + * so we can not use icmp_send and IPCB here.
> + */
> +
> + memset(opt, 0, sizeof(struct ip_options));
> + opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
Hmm, I think the above calculation should take into account the actual
length of the IP options, and not just the max size (calculate it
based on iphdr->ihl).
Beyond that fix, I think it's time to put together a proper patchset
and post it to the lists for formal review/merging.
Thanks for your work on this.
> + memcpy(opt->__data, (unsigned char *)&(ip_hdr(skb)[1]), opt->optlen);
> + if (ip_options_compile(dev_net(skb->dev), opt, NULL))
> + return;
> +
> if (gateway)
> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
> else
> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
> }
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs
From: John David Anglin @ 2019-01-25 16:30 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Russell King, Vivien Didelot, Florian Fainelli, netdev
In-Reply-To: <20190123002240.GF3634@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]
On 2019-01-22 7:22 p.m., Andrew Lunn wrote:
> >From my Espressobin
>
> cat /proc/interrupts
> ...
> 44: 0 0 mv88e6xxx-g1 3 Edge mv88e6xxx-g1-atu-prob
> 46: 0 0 mv88e6xxx-g1 5 Edge mv88e6xxx-g1-vtu-prob
> 48: 38 24 mv88e6xxx-g1 7 Edge mv88e6xxx-g2
> 51: 0 1 mv88e6xxx-g2 1 Edge !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:11
> 52: 0 0 mv88e6xxx-g2 2 Edge !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:12
> 53: 38 23 mv88e6xxx-g2 3 Edge !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:13
>
> These are PHY interrupts.
>
Thanks. That was the clue. I had tried to enable hardware for support
for switch interrupts. However,
the espressobin connects the switch interrupt output to a southbridge
pin that only supports edge interrupts.
Link detection works okay once the attached change is removed. I may
try and see if moving the interrupt
to a northbridge pin works.
I have to wonder about the use of edge interrupts. Given that a GPIO
interrupt isn't defined for the mv88e6xxx,
then the interrupt must be done by polling the global1 control register.
This also makes me wonder about this SD interrupt:
43: 0 0 GPIO1 3 Edge d00d0000.sdhci cd
Many poeple have trouble with SD cards on reboot.
Although the driver doesn't handle AVB interrupts, I started down this
path to try get ptp4l working better. I
have tweaked the polling but there are still circumstances where
timestamps are overwritten (or not written).
Dave
--
John David Anglin dave.anglin@bell.net
[-- Attachment #2: armada-3720-espressobin.dts-2.d --]
[-- Type: text/plain, Size: 698 bytes --]
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
index 3ab25ad402b9..65a3ff4da610 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
@@ -126,6 +126,14 @@
dsa,member = <0 0>;
+ interrupt-parent = <&gpiosb>;
+ /* Actually, the irq is active low but gpiosb doesn't
+ support that. Falling edge seems to work okay.
+ We need this for ptp support */
+ interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
ports {
#address-cells = <1>;
#size-cells = <0>;
^ permalink raw reply related
* Re: [PATCH v2 07/29] ARM: add kexec_file_load system call number
From: Russell King - ARM Linux admin @ 2019-01-25 16:21 UTC (permalink / raw)
To: Catalin Marinas
Cc: Arnd Bergmann, y2038, linux-api, linux-kernel, linux-arch,
mattst88, will.deacon, tony.luck, fenghua.yu, geert, monstr,
paul.burton, deller, benh, mpe, schwidefsky, heiko.carstens,
dalias, davem, luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm,
deepa.kernel, ebiederm, firoz.khan, linux-alpha, linux-arm-kernel,
linux-ia64, linux-m68k, linux-mips, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, netdev, linux-fsdevel
In-Reply-To: <20190125154359.GG25901@arrakis.emea.arm.com>
On Fri, Jan 25, 2019 at 03:43:59PM +0000, Catalin Marinas wrote:
> On Fri, Jan 18, 2019 at 05:18:13PM +0100, Arnd Bergmann wrote:
> > diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> > index 86de9eb34296..20ed7e026723 100644
> > --- a/arch/arm/tools/syscall.tbl
> > +++ b/arch/arm/tools/syscall.tbl
> > @@ -415,3 +415,4 @@
> > 398 common rseq sys_rseq
> > 399 common io_pgetevents sys_io_pgetevents
> > 400 common migrate_pages sys_migrate_pages
> > +401 common kexec_file_load sys_kexec_file_load
>
> I presume on arm32 this would still return -ENOSYS.
Yes, I already checked. If CONFIG_KEXEC_FILE is not set, then
kernel/kexec_file.c is not built, and we fall back to the stub
in kernel/sys_ni.c.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Jann Horn @ 2019-01-25 16:18 UTC (permalink / raw)
To: paulmck
Cc: Alexei Starovoitov, Peter Zijlstra, Alexei Starovoitov,
David S. Miller, Daniel Borkmann, jakub.kicinski,
Network Development, kernel-team, Ingo Molnar, Will Deacon
In-Reply-To: <20190125041152.GA4240@linux.ibm.com>
On Fri, Jan 25, 2019 at 5:12 AM Paul E. McKenney <paulmck@linux.ibm.com> wrote:
> On Fri, Jan 25, 2019 at 02:46:55AM +0100, Jann Horn wrote:
> > On Fri, Jan 25, 2019 at 2:22 AM Paul E. McKenney <paulmck@linux.ibm.com> wrote:
> > > On Thu, Jan 24, 2019 at 04:05:16PM -0800, Alexei Starovoitov wrote:
> > > > On Thu, Jan 24, 2019 at 03:42:32PM -0800, Paul E. McKenney wrote:
> > > > > On Thu, Jan 24, 2019 at 07:56:52PM +0100, Peter Zijlstra wrote:
> > > > > > On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> > > > > > >
> > > > > > > Thanks for having kernel/locking people on Cc...
> > > > > > >
> > > > > > > On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:
> > > > > > >
> > > > > > > > Implementation details:
> > > > > > > > - on !SMP bpf_spin_lock() becomes nop
> > > > > > >
> > > > > > > Because no BPF program is preemptible? I don't see any assertions or
> > > > > > > even a comment that says this code is non-preemptible.
> > > > > > >
> > > > > > > AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
> > > > > > > which is not sufficient.
> > > > > > >
> > > > > > > > - on architectures that don't support queued_spin_lock trivial lock is used.
> > > > > > > > Note that arch_spin_lock cannot be used, since not all archs agree that
> > > > > > > > zero == unlocked and sizeof(arch_spinlock_t) != sizeof(__u32).
> > > > > > >
> > > > > > > I really don't much like direct usage of qspinlock; esp. not as a
> > > > > > > surprise.
> > > > >
> > > > > Substituting the lightweight-reader SRCU as discussed earlier would allow
> > > > > use of a more generic locking primitive, for example, one that allowed
> > > > > blocking, at least in cases were the context allowed this.
> > > > >
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
> > > > > branch srcu-lr.2019.01.16a.
> > > > >
> > > > > One advantage of a more generic locking primitive would be keeping BPF
> > > > > programs independent of internal changes to spinlock primitives.
> > > >
> > > > Let's keep "srcu in bpf" discussion separate from bpf_spin_lock discussion.
> > > > bpf is not switching to srcu any time soon.
> > > > If/when it happens it will be only for certain prog+map types
> > > > like bpf syscall probes that need to be able to do copy_from_user
> > > > from bpf prog.
> > >
> > > Hmmm... What prevents BPF programs from looping infinitely within an
> > > RCU reader, and as you noted, preemption disabled?
> > >
> > > If BPF programs are in fact allowed to loop infinitely, it would be
> > > very good for the health of the kernel to have preemption enabled.
> > > And to be within an SRCU read-side critical section instead of an RCU
> > > read-side critical section.
> >
> > The BPF verifier prevents loops; this is in push_insn() in
> > kernel/bpf/verifier.c, which errors out with -EINVAL when a back edge
> > is encountered. For non-root programs, that limits the maximum number
> > of instructions per eBPF engine execution to
> > BPF_MAXINSNS*MAX_TAIL_CALL_CNT==4096*32==131072 (but that includes
> > call instructions, which can cause relatively expensive operations
> > like hash table lookups). For programs created with CAP_SYS_ADMIN,
> > things get more tricky because you can create your own functions and
> > call them repeatedly; I'm not sure whether the pessimal runtime there
> > becomes exponential, or whether there is some check that catches this.
>
> Whew!!! ;-)
>
> So no more than (say) 100 milliseconds?
Depends on RLIMIT_MEMLOCK and on how hard userspace is trying to make
things slow, I guess - if userspace manages to create a hashtable,
with a few dozen megabytes in size, with worst-case assignment of
elements to buckets (everything in a single bucket), every lookup call
on that bucket becomes a linked list traversal through a list that
must be stored in main memory because it's too big for the CPU caches.
I don't know into how much time that translates.
^ permalink raw reply
* [PATCH net-next] tcp: allow zerocopy with fastopen
From: Willem de Bruijn @ 2019-01-25 16:17 UTC (permalink / raw)
To: netdev; +Cc: davem, ycheng, edumazet, alexey.kodanev, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Accept MSG_ZEROCOPY in all the TCP states that allow sendmsg. Remove
the explicit check for ESTABLISHED and CLOSE_WAIT states.
This requires correctly handling zerocopy state (uarg, sk_zckey) in
all paths reachable from other TCP states. Such as the EPIPE case
in sk_stream_wait_connect, which a sendmsg() in incorrect state will
now hit. Most paths are already safe.
Only extension needed is for TCP Fastopen active open. This can build
an skb with data in tcp_send_syn_data. Pass the uarg along with other
fastopen state, so that this skb also generates a zerocopy
notification on release.
Tested with active and passive tcp fastopen packetdrill scripts at
https://github.com/wdebruij/packetdrill/commit/1747eef03d25a2404e8132817d0f1244fd6f129d
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/net/tcp.h | 1 +
net/ipv4/tcp.c | 11 ++++-------
net/ipv4/tcp_output.c | 1 +
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 5c950180d61be..a6e0355921e1d 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1608,6 +1608,7 @@ struct tcp_fastopen_request {
struct msghdr *data; /* data in MSG_FASTOPEN */
size_t size;
int copied; /* queued in tcp_connect() */
+ struct ubuf_info *uarg;
};
void tcp_free_fastopen_req(struct tcp_sock *tp);
void tcp_fastopen_destroy_cipher(struct sock *sk);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5f099c9d04e5d..12ba21433dd00 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1127,7 +1127,8 @@ void tcp_free_fastopen_req(struct tcp_sock *tp)
}
static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
- int *copied, size_t size)
+ int *copied, size_t size,
+ struct ubuf_info *uarg)
{
struct tcp_sock *tp = tcp_sk(sk);
struct inet_sock *inet = inet_sk(sk);
@@ -1147,6 +1148,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
return -ENOBUFS;
tp->fastopen_req->data = msg;
tp->fastopen_req->size = size;
+ tp->fastopen_req->uarg = uarg;
if (inet->defer_connect) {
err = tcp_connect(sk);
@@ -1186,11 +1188,6 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
flags = msg->msg_flags;
if (flags & MSG_ZEROCOPY && size && sock_flag(sk, SOCK_ZEROCOPY)) {
- if ((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) {
- err = -EINVAL;
- goto out_err;
- }
-
skb = tcp_write_queue_tail(sk);
uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb));
if (!uarg) {
@@ -1205,7 +1202,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
if (unlikely(flags & MSG_FASTOPEN || inet_sk(sk)->defer_connect) &&
!tp->repair) {
- err = tcp_sendmsg_fastopen(sk, msg, &copied_syn, size);
+ err = tcp_sendmsg_fastopen(sk, msg, &copied_syn, size, uarg);
if (err == -EINPROGRESS && copied_syn > 0)
goto out;
else if (err)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6527f61f59ff1..26a2948dca954 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3455,6 +3455,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
skb_trim(syn_data, copied);
space = copied;
}
+ skb_zcopy_set(syn_data, fo->uarg, NULL);
}
/* No more data pending in inet_wait_for_connect() */
if (space == fo->size)
--
2.20.1.495.gaa96b0ce6b-goog
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock
From: Paul E. McKenney @ 2019-01-25 16:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Jann Horn, Peter Zijlstra, Alexei Starovoitov,
David S. Miller, Daniel Borkmann, jakub.kicinski@netronome.com,
Network Development, Kernel Team, Ingo Molnar, Will Deacon
In-Reply-To: <865e3302-6f7f-d20b-07ea-e387b0a7a570@fb.com>
On Fri, Jan 25, 2019 at 04:47:20AM +0000, Alexei Starovoitov wrote:
> On 1/24/19 8:31 PM, Paul E. McKenney wrote:
> > On Fri, Jan 25, 2019 at 04:27:02AM +0000, Alexei Starovoitov wrote:
> >> On 1/24/19 6:38 PM, Alexei Starovoitov wrote:
> >>>> For programs created with CAP_SYS_ADMIN,
> >>>> things get more tricky because you can create your own functions and
> >>>> call them repeatedly; I'm not sure whether the pessimal runtime there
> >>>> becomes exponential, or whether there is some check that catches this.
> >>> I think you're referring to bpf-to-bpf calls.
> >>> The limit it still the same. 4k per program including all calls.
> >>> tail calls are not allowed when bpf-to-bpf is used. So no 32 multiplier.
> >>
> >> Jann,
> >>
> >> I think you meant
> >> main:
> >> call A
> >> call A
> >> call A
> >> exit
> >> A:
> >> call B
> >> call B
> >> call B
> >> exit
> >> B:
> >> call C
> >> ...
> >>
> >> scenario when everything fits into 4k?
> >> Would be great if you can construct such test while we're fixing
> >> the rest of the issues brought up in this thread.
> >> It will definitely be no more than BPF_COMPLEXITY_LIMIT_INSNS
> >> which is 128k, but I wonder what will be the actual number of
> >> executed insns.
> >> I think such clever constructed sequence can actually
> >> hit 128k executed too.
> >> It would be awesome test to add to test_verifier.c
> >> We have some of such pushing-the-boundary tests in lib/test_bpf.c
> >> that are generated in assembler.
> >> The longest takes 23853 nanoseconds, but clever bpf2bpf call hack
> >> like above with map_update call in the leaf function should
> >> certainly take much longer.
> >> I accept Paul's challenge to try to get such fancy bpf prog
> >> to take 100 millseconds :)
> >
> > Fair enough! But once you meet my challenge, the RCU CPU stall warning
> > code will challenge you to hit 21 seconds (or only three seconds given
> > an appropriately configured kernel). ;-)
>
> if (till_stall_check < 3) {
> WRITE_ONCE(rcu_cpu_stall_timeout, 3);
> till_stall_check = 3;
>
> let's change that limit to 1 !
Heh! It was 1.5 seconds back in DYNIX/ptx. However, taking it below
3 seconds would require some other adjustments. Something about there
being a lot more moving parts in the Linux kernel.
> Seriously though folks have proposed to teach bpf verifier
> to sprinkle cond_resched() automatically into bpf program
> when critical path through the program reaches certain insn limit.
> The verifier can easily be taught to compute the longest path.
Good point, for PREEMPT=n, cond_resched() will help.
> Other folks proposed to get rid of 4k limit when prog
> is preemptable and executing in user context.
> That's when srcu will come into play.
OK, please let me know when you get to this point so that I can get
the lightweight variant of SRCU moving forward.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 29/29] y2038: add 64-bit time_t syscalls to all 32-bit architectures
From: Catalin Marinas @ 2019-01-25 15:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, linux-api, linux-kernel, linux-arch, mattst88, linux,
will.deacon, tony.luck, fenghua.yu, geert, monstr, paul.burton,
deller, benh, mpe, schwidefsky, heiko.carstens, dalias, davem,
luto, tglx, mingo, hpa, x86, jcmvbkbc, akpm, deepa.kernel,
ebiederm, firoz.khan, linux-alpha, linux-arm-kernel, linux-ia64,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, netdev, linux-fsdevel
In-Reply-To: <20190118161835.2259170-30-arnd@arndb.de>
On Fri, Jan 18, 2019 at 05:18:35PM +0100, Arnd Bergmann wrote:
> This adds 21 new system calls on each ABI that has 32-bit time_t
> today. All of these have the exact same semantics as their existing
> counterparts, and the new ones all have macro names that end in 'time64'
> for clarification.
>
> This gets us to the point of being able to safely use a C library
> that has 64-bit time_t in user space. There are still a couple of
> loose ends to tie up in various areas of the code, but this is the
> big one, and should be entirely uncontroversial at this point.
>
> In particular, there are four system calls (getitimer, setitimer,
> waitid, and getrusage) that don't have a 64-bit counterpart yet,
> but these can all be safely implemented in the C library by wrapping
> around the existing system calls because the 32-bit time_t they
> pass only counts elapsed time, not time since the epoch. They
> will be dealt with later.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
(as long as compat follows the arm32 syscall numbers)
^ 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