* Re: [PATCH 3/3] net: dsa: ksz: Add Microchip KSZ8795 DSA driver
From: Andrew Lunn @ 2019-07-25 14:03 UTC (permalink / raw)
To: Marek Vasut
Cc: netdev, Tristram Ha, David S . Miller, Florian Fainelli,
Vivien Didelot, Woojung Huh
In-Reply-To: <20190724134048.31029-4-marex@denx.de>
On Wed, Jul 24, 2019 at 03:40:48PM +0200, Marek Vasut wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> +static void ksz8795_phy_setup(struct ksz_device *dev, int port,
> + struct phy_device *phy)
> +{
> + if (port < dev->phy_port_cnt) {
> + /*
> + * SUPPORTED_Asym_Pause and SUPPORTED_Pause can be removed to
> + * disable flow control when rate limiting is used.
> + */
> + linkmode_copy(phy->advertising, phy->supported);
> + }
> +}
Hi Marek
Do you know why this is needed?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH] ipip: validate header length in ipip_tunnel_xmit
From: Willem de Bruijn @ 2019-07-25 13:48 UTC (permalink / raw)
To: Haishuang Yan
Cc: David S. Miller, Alexey Kuznetsov, Network Development,
linux-kernel
In-Reply-To: <1564024076-13764-2-git-send-email-yanhaishuang@cmss.chinamobile.com>
On Wed, Jul 24, 2019 at 11:09 PM Haishuang Yan
<yanhaishuang@cmss.chinamobile.com> wrote:
>
> We need the same checks introduced by commit cb9f1b783850
> ("ip: validate header length on virtual device xmit") for
> ipip tunnel.
Fixes: cb9f1b783850b ("ip: validate header length on virtual device xmit")
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Good catch. reg_vif_xmit in net/ipv4/ipmr.c probably also needs it.
All other ndo_start_xmit under net/ipv4 and net/ipv6 have this check
as of the above commit.
^ permalink raw reply
* Re: [PATCH v6 0/5] net: macb: cover letter
From: Andrew Lunn @ 2019-07-25 13:48 UTC (permalink / raw)
To: Parshuram Raju Thombare
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, linux@armlinux.org.uk,
netdev@vger.kernel.org, hkallweit1@gmail.com,
linux-kernel@vger.kernel.org, Rafal Ciepiela, Piotr Sroka,
Anil Joy Varughese, Arthur Marris, Steven Ho, Milind Parab
In-Reply-To: <CO2PR07MB246961335F7D401785377765C1C10@CO2PR07MB2469.namprd07.prod.outlook.com>
On Thu, Jul 25, 2019 at 01:27:58PM +0000, Parshuram Raju Thombare wrote:
> Hi Andrew,
>
> >One thing which was never clear is how you are testing the features you are
> >adding. Please could you describe your test setup and how each new feature
> >is tested using that hardware. I'm particularly interested in what C45 device
> >are you using? But i expect Russell would like to know more about SFP
> >modules you are using. Do you have any which require 1000BaseX,
> >2500BaseX, or provide copper 1G?
>
> Sorry for late reply.
> Here is a little more information on our setup used for testing C45 patch with a view to
> try clarify a few points.
> Regarding the MDIO communication channel that our controller supports - We have tested
> MDIO transfers through Clause 22, but none of our local PHY's support Clause 45 so our hardware
> team have created an example Clause 45 slave device for us to add support to the driver.
O.K.
Given Russells reply, i suggest you submit the MDIO Clause 45 patch,
and throw all the other patches away.
Andrew
^ permalink raw reply
* Re: [PATCH 15/22] docs: index.rst: don't use genindex for pdf output
From: Vinod Koul @ 2019-07-25 13:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Jonathan Corbet, Sanyog Kale, Pierre-Louis Bossart,
David S. Miller, Jaroslav Kysela, Takashi Iwai, linux-doc,
dmaengine, alsa-devel, netdev
In-Reply-To: <45d57666e5738a0b85e948b0e94151fe1b1f9274.1563792334.git.mchehab+samsung@kernel.org>
On 22-07-19, 08:07, Mauro Carvalho Chehab wrote:
> The genindex logic is meant to be used only for html output, as
> pdf build has its own way to generate indexes.
> Documentation/driver-api/dmaengine/index.rst | 2 +-
> Documentation/driver-api/soundwire/index.rst | 2 +-
For dmaengine and soundwire:
Acked-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
^ permalink raw reply
* Re: [PATCH] net: dsa: Check existence of .port_mdb_add callback before calling it
From: Andrew Lunn @ 2019-07-25 13:42 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Vivien Didelot, Florian Fainelli, David S . Miller, Chen-Yu Tsai,
netdev, linux-kernel
In-Reply-To: <20190724034702.21212-1-wens@kernel.org>
On Wed, Jul 24, 2019 at 11:47:02AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> With the recent addition of commit 75dad2520fc3 ("net: dsa: b53: Disable
> all ports on setup"), users of b53 (BCM53125 on Lamobo R1 in my case)
> are forced to use the dsa subsystem to enable the switch, instead of
> having it in the default transparent "forward-to-all" mode.
Hi Chen-Yu
I would do the check earlier, at the beginning of
dsa_switch_mdb_add(). It is then similar to dsa_switch_mdb_del().
Andrew
^ permalink raw reply
* Re: [PATCH 2/7] can: rcar_canfd: fix possible IRQ storm on high load
From: Marc Kleine-Budde @ 2019-07-25 13:41 UTC (permalink / raw)
To: Nikita Yushchenko, Sasha Levin, netdev; +Cc: davem, linux-can, linux-stable
In-Reply-To: <6b23b091-92d1-b05d-b451-d8c78a990ef3@cogentembedded.com>
[-- Attachment #1.1: Type: text/plain, Size: 607 bytes --]
On 7/25/19 3:29 PM, Nikita Yushchenko wrote:
>> NOTE: The patch will not be queued to stable trees until it is upstream.
>>
>> How should we proceed with this patch?
>
> I don't know.
> Maintainer did not respond, nor to original send nor to resend.
You can backport this patch to v4.9.186 and send it to Sasha
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] net: hns: fix LED configuration for marvell phy
From: liuyonglong @ 2019-07-25 13:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, netdev, linux-kernel, linuxarm, salil.mehta,
yisen.zhuang, shiju.jose
In-Reply-To: <20190725130807.GB21952@lunn.ch>
On 2019/7/25 21:08, Andrew Lunn wrote:
>> You are discussing about the DT configuration, is Matthias Kaehlcke's work
>> also provide a generic way to configure PHY LEDS using ACPI?
>
> In general, you should be able to use the same properties in ACPI as
> DT. If the device_property_read_X() API is used, it will try both ACPI
> and OF to get the property.
>
> Andrew
>
> .
>
OK, thanks very much!
^ permalink raw reply
* Re: [PATCH v6 0/5] net: macb: cover letter
From: Russell King - ARM Linux admin @ 2019-07-25 13:36 UTC (permalink / raw)
To: Parshuram Raju Thombare
Cc: Andrew Lunn, nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, netdev@vger.kernel.org,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Anil Joy Varughese, Arthur Marris,
Steven Ho, Milind Parab
In-Reply-To: <CO2PR07MB246961335F7D401785377765C1C10@CO2PR07MB2469.namprd07.prod.outlook.com>
On Thu, Jul 25, 2019 at 01:27:58PM +0000, Parshuram Raju Thombare wrote:
> Hi Andrew,
>
> >One thing which was never clear is how you are testing the features you are
> >adding. Please could you describe your test setup and how each new feature
> >is tested using that hardware. I'm particularly interested in what C45 device
> >are you using? But i expect Russell would like to know more about SFP
> >modules you are using. Do you have any which require 1000BaseX,
> >2500BaseX, or provide copper 1G?
>
> Sorry for late reply.
> Here is a little more information on our setup used for testing C45 patch with a view to
> try clarify a few points.
> Regarding the MDIO communication channel that our controller supports - We have tested
> MDIO transfers through Clause 22, but none of our local PHY's support Clause 45 so our hardware
> team have created an example Clause 45 slave device for us to add support to the driver.
> Note our hardware has been in silicon for 20 years, with customers using their own software to support
> MDIO (both clause 22 and clause 45 functionality) and so this has been in Cadence's hardware controller
> many times.
> The programming interface is not hugely different between the two clauses and therefore we feel the risk is low.
>
> For other features like SGMII, USXGMII we are using kc705 and vcu118 FPGA boards.
> 10G SFP+ module from Tyco electronics is used for testing 10G USXGMII in fixed AN mode.
SFP and SFP+ modules take SGMII, 1000BASE-X, possibly 2500BASE-X and
10GBASE-R all over a single serdes lane.
USXGMII might be used from the MAC to some sort of PHY which then
converts to 10GBASE-R.
If you have a PHY present, then using phylink and trying to link the
MAC directly with the SFP cage in software is the _wrong approach_.
I've stated this several times.
I'm getting to the point of asking you not to persist with your use
of phylink with your driver - I do not believe that your hardware has
any justification for its use, and I also believe that your use of
phylink is positively hurtful to the long term maintenance of phylink
itself.
In other words, you persisting to (ab)use phylink _hurts_ our ability
to maintain it into the future.
I'm also at the point where I'm giving up reviewing your patches - you
don't seem to take the issues I raise on board at all, so I feel like
I'm completely wasting my time trying to get you to make improvements.
Thanks.
--
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 net-next 13/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-07-25 13:35 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem
In-Reply-To: <20190722214023.9513-14-snelson@pensando.io>
> +static int ionic_get_module_eeprom(struct net_device *netdev,
> + struct ethtool_eeprom *ee,
> + u8 *data)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + struct ionic_dev *idev = &lif->ionic->idev;
> + struct xcvr_status *xcvr;
> + u32 len;
> +
> + /* The NIC keeps the module prom up-to-date in the DMA space
> + * so we can simply copy the module bytes into the data buffer.
> + */
> + xcvr = &idev->port_info->status.xcvr;
> + len = min_t(u32, sizeof(xcvr->sprom), ee->len);
> + memcpy(data, xcvr->sprom, len);
> +
> + return 0;
> +}
Is the firmware doing this DMA update atomically? The diagnostic
values are u16s. Is there any chance we do this memcpy at the same
time the DMA is active and we get a mix of old and new data?
Often in cases like this you do the copy twice and ensure you get the
same values each time. If not, keep repeating the copy until you do
get the same values twice.
Andrew
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-25 13:06 UTC (permalink / raw)
To: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel,
allan.nielsen
In-Reply-To: <1564055044-27593-1-git-send-email-horatiu.vultur@microchip.com>
On 25/07/2019 14:44, Horatiu Vultur wrote:
> There is no way to configure the bridge, to receive only specific link
> layer multicast addresses. From the description of the command 'bridge
> fdb append' is supposed to do that, but there was no way to notify the
> network driver that the bridge joined a group, because LLADDR was added
> to the unicast netdev_hw_addr_list.
>
> Therefore update fdb_add_entry to check if the NLM_F_APPEND flag is set
> and if the source is NULL, which represent the bridge itself. Then add
> address to multicast netdev_hw_addr_list for each bridge interfaces.
> And then the .ndo_set_rx_mode function on the driver is called. To notify
> the driver that the list of multicast mac addresses changed.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> net/bridge/br_fdb.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
Hi,
I'm sorry but this patch is wrong on many levels, some notes below. In general
NLM_F_APPEND is only used in vxlan, the bridge does not handle that flag at all.
FDB is only for *unicast*, nothing is joined and no multicast should be used with fdbs.
MDB is used for multicast handling, but both of these are used for forwarding.
The reason the static fdbs are added to the filter is for non-promisc ports, so they can
receive traffic destined for these FDBs for forwarding.
If you'd like to join any multicast group please use the standard way, if you'd like to join
it only on a specific port - join it only on that port (or ports) and the bridge and you'll
have the effect that you're describing. What do you mean there's no way ?
In addition you're allowing a mix of mcast functions to be called with unicast addresses
and vice versa, it is not that big of a deal because the kernel will simply return an error
but still makes no sense.
Nacked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index b1d3248..d93746d 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -175,6 +175,29 @@ static void fdb_add_hw_addr(struct net_bridge *br, const unsigned char *addr)
> }
> }
>
> +static void fdb_add_hw_maddr(struct net_bridge *br, const unsigned char *addr)
> +{
> + int err;
> + struct net_bridge_port *p;
> +
> + ASSERT_RTNL();
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (!br_promisc_port(p)) {
> + err = dev_mc_add(p->dev, addr);
> + if (err)
> + goto undo;
> + }
> + }
> +
> + return;
> +undo:
> + list_for_each_entry_continue_reverse(p, &br->port_list, list) {
> + if (!br_promisc_port(p))
> + dev_mc_del(p->dev, addr);
> + }
> +}
> +
> /* When a static FDB entry is deleted, the HW address from that entry is
> * also removed from the bridge private HW address list and updates all
> * the ports with needed information.
> @@ -192,13 +215,27 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
> }
> }
>
> +static void fdb_del_hw_maddr(struct net_bridge *br, const unsigned char *addr)
> +{
> + struct net_bridge_port *p;
> +
> + ASSERT_RTNL();
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (!br_promisc_port(p))
> + dev_mc_del(p->dev, addr);
> + }
> +}
> +
> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
> bool swdev_notify)
> {
> trace_fdb_delete(br, f);
>
> - if (f->is_static)
> + if (f->is_static) {
> fdb_del_hw_addr(br, f->key.addr.addr);
> + fdb_del_hw_maddr(br, f->key.addr.addr);
Walking over all ports again for each static delete is a no-go.
> + }
>
> hlist_del_init_rcu(&f->fdb_node);
> rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode,
> @@ -843,13 +880,19 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> fdb->is_local = 1;
> if (!fdb->is_static) {
> fdb->is_static = 1;
> - fdb_add_hw_addr(br, addr);
> + if (flags & NLM_F_APPEND && !source)
> + fdb_add_hw_maddr(br, addr);
> + else
> + fdb_add_hw_addr(br, addr);
> }
> } else if (state & NUD_NOARP) {
> fdb->is_local = 0;
> if (!fdb->is_static) {
> fdb->is_static = 1;
> - fdb_add_hw_addr(br, addr);
> + if (flags & NLM_F_APPEND && !source)
> + fdb_add_hw_maddr(br, addr);
> + else
> + fdb_add_hw_addr(br, addr);
> }
> } else {
> fdb->is_local = 0;
>
^ permalink raw reply
* [PATCH net] ocelot: Cancel delayed work before wq destruction
From: Claudiu Manoil @ 2019-07-25 13:33 UTC (permalink / raw)
To: David S . Miller; +Cc: Alexandre Belloni, netdev, UNGLinuxDriver
Make sure the delayed work for stats update is not pending before
wq destruction.
This fixes the module unload path.
The issue is there since day 1.
Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
drivers/net/ethernet/mscc/ocelot.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index b71e4ecbe469..6932e615d4b0 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1818,6 +1818,7 @@ EXPORT_SYMBOL(ocelot_init);
void ocelot_deinit(struct ocelot *ocelot)
{
+ cancel_delayed_work(&ocelot->stats_work);
destroy_workqueue(ocelot->stats_queue);
mutex_destroy(&ocelot->stats_lock);
ocelot_ace_deinit();
--
2.17.1
^ permalink raw reply related
* [RFC PATCH] rxrpc: Fix -Wframe-larger-than= warnings from on-stack crypto
From: David Howells @ 2019-07-25 13:31 UTC (permalink / raw)
To: linux-afs; +Cc: Arnd Bergmann, Herbert Xu, dhowells, linux-crypto, netdev
rxkad sometimes triggers a warning about oversized stack frames when
building with clang for a 32-bit architecture:
net/rxrpc/rxkad.c:243:12: error: stack frame size of 1088 bytes in function 'rxkad_secure_packet' [-Werror,-Wframe-larger-than=]
net/rxrpc/rxkad.c:501:12: error: stack frame size of 1088 bytes in function 'rxkad_verify_packet' [-Werror,-Wframe-larger-than=]
The problem is the combination of SYNC_SKCIPHER_REQUEST_ON_STACK() in
rxkad_verify_packet()/rxkad_secure_packet() with the relatively large
scatterlist in rxkad_verify_packet_1()/rxkad_secure_packet_encrypt().
The warning does not show up when using gcc, which does not inline the
functions as aggressively, but the problem is still the same.
Allocate the cipher buffers from the slab instead, caching the allocated
packet crypto request memory used for DATA packet crypto in the rxrpc_call
struct.
Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Herbert Xu <herbert@gondor.apana.org.au>
---
net/rxrpc/ar-internal.h | 4 ++
net/rxrpc/call_object.c | 4 +-
net/rxrpc/insecure.c | 5 ++
net/rxrpc/rxkad.c | 103 ++++++++++++++++++++++++++++++++++++++---------
4 files changed, 96 insertions(+), 20 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 80335b4ee4fd..bea2a02850af 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -226,6 +226,9 @@ struct rxrpc_security {
int (*verify_packet)(struct rxrpc_call *, struct sk_buff *,
unsigned int, unsigned int, rxrpc_seq_t, u16);
+ /* Free crypto request on a call */
+ void (*free_call_crypto)(struct rxrpc_call *);
+
/* Locate the data in a received packet that has been verified. */
void (*locate_data)(struct rxrpc_call *, struct sk_buff *,
unsigned int *, unsigned int *);
@@ -557,6 +560,7 @@ struct rxrpc_call {
unsigned long expect_term_by; /* When we expect call termination by */
u32 next_rx_timo; /* Timeout for next Rx packet (jif) */
u32 next_req_timo; /* Timeout for next Rx request packet (jif) */
+ struct skcipher_request *cipher_req; /* Packet cipher request buffer */
struct timer_list timer; /* Combined event timer */
struct work_struct processor; /* Event processor */
rxrpc_notify_rx_t notify_rx; /* kernel service Rx notification function */
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 217b12be9e08..60cbc81dc461 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -476,8 +476,10 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)
_debug("RELEASE CALL %p (%d CONN %p)", call, call->debug_id, conn);
- if (conn)
+ if (conn) {
rxrpc_disconnect_call(call);
+ conn->security->free_call_crypto(call);
+ }
for (i = 0; i < RXRPC_RXTX_BUFF_SIZE; i++) {
rxrpc_free_skb(call->rxtx_buffer[i],
diff --git a/net/rxrpc/insecure.c b/net/rxrpc/insecure.c
index a29d26c273b5..f6c59f5fae9d 100644
--- a/net/rxrpc/insecure.c
+++ b/net/rxrpc/insecure.c
@@ -33,6 +33,10 @@ static int none_verify_packet(struct rxrpc_call *call, struct sk_buff *skb,
return 0;
}
+static void none_free_call_crypto(struct rxrpc_call *call)
+{
+}
+
static void none_locate_data(struct rxrpc_call *call, struct sk_buff *skb,
unsigned int *_offset, unsigned int *_len)
{
@@ -83,6 +87,7 @@ const struct rxrpc_security rxrpc_no_security = {
.exit = none_exit,
.init_connection_security = none_init_connection_security,
.prime_packet_security = none_prime_packet_security,
+ .free_call_crypto = none_free_call_crypto,
.secure_packet = none_secure_packet,
.verify_packet = none_verify_packet,
.locate_data = none_locate_data,
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index ae8cd8926456..dbb109da1835 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -43,6 +43,7 @@ struct rxkad_level2_hdr {
* packets
*/
static struct crypto_sync_skcipher *rxkad_ci;
+static struct skcipher_request *rxkad_ci_req;
static DEFINE_MUTEX(rxkad_ci_mutex);
/*
@@ -99,8 +100,8 @@ static int rxkad_init_connection_security(struct rxrpc_connection *conn)
*/
static int rxkad_prime_packet_security(struct rxrpc_connection *conn)
{
+ struct skcipher_request *req;
struct rxrpc_key_token *token;
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, conn->cipher);
struct scatterlist sg;
struct rxrpc_crypt iv;
__be32 *tmpbuf;
@@ -115,6 +116,12 @@ static int rxkad_prime_packet_security(struct rxrpc_connection *conn)
if (!tmpbuf)
return -ENOMEM;
+ req = skcipher_request_alloc(&conn->cipher->base, GFP_NOFS);
+ if (!req) {
+ kfree(tmpbuf);
+ return -ENOMEM;
+ }
+
token = conn->params.key->payload.data[0];
memcpy(&iv, token->kad->session_key, sizeof(iv));
@@ -128,7 +135,7 @@ static int rxkad_prime_packet_security(struct rxrpc_connection *conn)
skcipher_request_set_callback(req, 0, NULL, NULL);
skcipher_request_set_crypt(req, &sg, &sg, tmpsize, iv.x);
crypto_skcipher_encrypt(req);
- skcipher_request_zero(req);
+ skcipher_request_free(req);
memcpy(&conn->csum_iv, tmpbuf + 2, sizeof(conn->csum_iv));
kfree(tmpbuf);
@@ -136,6 +143,35 @@ static int rxkad_prime_packet_security(struct rxrpc_connection *conn)
return 0;
}
+/*
+ * Allocate and prepare the crypto request on a call. For any particular call,
+ * this is called serially for the packets, so no lock should be necessary.
+ */
+static struct skcipher_request *rxkad_get_call_crypto(struct rxrpc_call *call)
+{
+ struct crypto_skcipher *tfm = &call->conn->cipher->base;
+ struct skcipher_request *cipher_req = call->cipher_req;
+
+ if (!cipher_req) {
+ cipher_req = skcipher_request_alloc(tfm, GFP_NOFS);
+ if (!cipher_req)
+ return NULL;
+ call->cipher_req = cipher_req;
+ }
+
+ return cipher_req;
+}
+
+/*
+ * Clean up the crypto on a call.
+ */
+static void rxkad_free_call_crypto(struct rxrpc_call *call)
+{
+ if (call->cipher_req)
+ skcipher_request_free(call->cipher_req);
+ call->cipher_req = NULL;
+}
+
/*
* partially encrypt a packet (level 1 security)
*/
@@ -246,7 +282,7 @@ static int rxkad_secure_packet(struct rxrpc_call *call,
void *sechdr)
{
struct rxrpc_skb_priv *sp;
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher);
+ struct skcipher_request *req;
struct rxrpc_crypt iv;
struct scatterlist sg;
u32 x, y;
@@ -265,6 +301,10 @@ static int rxkad_secure_packet(struct rxrpc_call *call,
if (ret < 0)
return ret;
+ req = rxkad_get_call_crypto(call);
+ if (!req)
+ return -ENOMEM;
+
/* continue encrypting from where we left off */
memcpy(&iv, call->conn->csum_iv.x, sizeof(iv));
@@ -502,7 +542,7 @@ static int rxkad_verify_packet(struct rxrpc_call *call, struct sk_buff *skb,
unsigned int offset, unsigned int len,
rxrpc_seq_t seq, u16 expected_cksum)
{
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, call->conn->cipher);
+ struct skcipher_request *req;
struct rxrpc_crypt iv;
struct scatterlist sg;
bool aborted;
@@ -515,6 +555,10 @@ static int rxkad_verify_packet(struct rxrpc_call *call, struct sk_buff *skb,
if (!call->conn->cipher)
return 0;
+ req = rxkad_get_call_crypto(call);
+ if (!req)
+ return -ENOMEM;
+
/* continue encrypting from where we left off */
memcpy(&iv, call->conn->csum_iv.x, sizeof(iv));
@@ -747,14 +791,18 @@ static void rxkad_calc_response_checksum(struct rxkad_response *response)
/*
* encrypt the response packet
*/
-static void rxkad_encrypt_response(struct rxrpc_connection *conn,
- struct rxkad_response *resp,
- const struct rxkad_key *s2)
+static int rxkad_encrypt_response(struct rxrpc_connection *conn,
+ struct rxkad_response *resp,
+ const struct rxkad_key *s2)
{
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, conn->cipher);
+ struct skcipher_request *req;
struct rxrpc_crypt iv;
struct scatterlist sg[1];
+ req = skcipher_request_alloc(&conn->cipher->base, GFP_NOFS);
+ if (!req)
+ return -ENOMEM;
+
/* continue encrypting from where we left off */
memcpy(&iv, s2->session_key, sizeof(iv));
@@ -764,7 +812,8 @@ static void rxkad_encrypt_response(struct rxrpc_connection *conn,
skcipher_request_set_callback(req, 0, NULL, NULL);
skcipher_request_set_crypt(req, sg, sg, sizeof(resp->encrypted), iv.x);
crypto_skcipher_encrypt(req);
- skcipher_request_zero(req);
+ skcipher_request_free(req);
+ return 0;
}
/*
@@ -839,8 +888,9 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
/* calculate the response checksum and then do the encryption */
rxkad_calc_response_checksum(resp);
- rxkad_encrypt_response(conn, resp, token->kad);
- ret = rxkad_send_response(conn, &sp->hdr, resp, token->kad);
+ ret = rxkad_encrypt_response(conn, resp, token->kad);
+ if (ret == 0)
+ ret = rxkad_send_response(conn, &sp->hdr, resp, token->kad);
kfree(resp);
return ret;
@@ -1017,18 +1067,16 @@ static void rxkad_decrypt_response(struct rxrpc_connection *conn,
struct rxkad_response *resp,
const struct rxrpc_crypt *session_key)
{
- SYNC_SKCIPHER_REQUEST_ON_STACK(req, rxkad_ci);
+ struct skcipher_request *req = rxkad_ci_req;
struct scatterlist sg[1];
struct rxrpc_crypt iv;
_enter(",,%08x%08x",
ntohl(session_key->n[0]), ntohl(session_key->n[1]));
- ASSERT(rxkad_ci != NULL);
-
mutex_lock(&rxkad_ci_mutex);
if (crypto_sync_skcipher_setkey(rxkad_ci, session_key->x,
- sizeof(*session_key)) < 0)
+ sizeof(*session_key)) < 0)
BUG();
memcpy(&iv, session_key, sizeof(iv));
@@ -1222,10 +1270,26 @@ static void rxkad_clear(struct rxrpc_connection *conn)
*/
static int rxkad_init(void)
{
+ struct crypto_sync_skcipher *tfm;
+ struct skcipher_request *req;
+
/* pin the cipher we need so that the crypto layer doesn't invoke
* keventd to go get it */
- rxkad_ci = crypto_alloc_sync_skcipher("pcbc(fcrypt)", 0, 0);
- return PTR_ERR_OR_ZERO(rxkad_ci);
+ tfm = crypto_alloc_sync_skcipher("pcbc(fcrypt)", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ req = skcipher_request_alloc(&tfm->base, GFP_KERNEL);
+ if (!req)
+ goto nomem_tfm;
+
+ rxkad_ci_req = req;
+ rxkad_ci = tfm;
+ return 0;
+
+nomem_tfm:
+ crypto_free_sync_skcipher(tfm);
+ return -ENOMEM;
}
/*
@@ -1233,8 +1297,8 @@ static int rxkad_init(void)
*/
static void rxkad_exit(void)
{
- if (rxkad_ci)
- crypto_free_sync_skcipher(rxkad_ci);
+ crypto_free_sync_skcipher(rxkad_ci);
+ skcipher_request_free(rxkad_ci_req);
}
/*
@@ -1249,6 +1313,7 @@ const struct rxrpc_security rxkad = {
.prime_packet_security = rxkad_prime_packet_security,
.secure_packet = rxkad_secure_packet,
.verify_packet = rxkad_verify_packet,
+ .free_call_crypto = rxkad_free_call_crypto,
.locate_data = rxkad_locate_data,
.issue_challenge = rxkad_issue_challenge,
.respond_to_challenge = rxkad_respond_to_challenge,
^ permalink raw reply related
* Re: [PATCH 2/7] can: rcar_canfd: fix possible IRQ storm on high load
From: Nikita Yushchenko @ 2019-07-25 13:29 UTC (permalink / raw)
To: Sasha Levin, Marc Kleine-Budde, netdev; +Cc: davem, linux-can, linux-stable
In-Reply-To: <20190724210328.D91DF21873@mail.kernel.org>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
I don't know.
Maintainer did not respond, nor to original send nor to resend.
^ permalink raw reply
* Re: [PATCH v2 0/2] mmc: core: Fix Marvell WiFi reset by adding SDIO API to replug card
From: Ulf Hansson @ 2019-07-25 13:28 UTC (permalink / raw)
To: Douglas Anderson
Cc: Kalle Valo, Adrian Hunter, Ganapathi Bhat, linux-wireless,
Andreas Fenkart, Brian Norris, Amitkumar Karwar,
open list:ARM/Rockchip SoC..., Wolfram Sang, Nishant Sarmukadam,
netdev, Avri Altman, linux-mmc@vger.kernel.org, David S. Miller,
Xinming Hu, Linux Kernel Mailing List, Thomas Gleixner,
Kate Stewart
In-Reply-To: <20190722193939.125578-1-dianders@chromium.org>
On Mon, 22 Jul 2019 at 21:41, Douglas Anderson <dianders@chromium.org> wrote:
>
> As talked about in the thread at:
>
> http://lkml.kernel.org/r/CAD=FV=X7P2F1k_zwHc0mbtfk55-rucTz_GoDH=PL6zWqKYcpuw@mail.gmail.com
>
> ...when the Marvell WiFi card tries to reset itself it kills
> Bluetooth. It was observed that we could re-init the card properly by
> unbinding / rebinding the host controller. It was also observed that
> in the downstream Chrome OS codebase the solution used was
> mmc_remove_host() / mmc_add_host(), which is similar to the solution
> in this series.
>
> So far I've only done testing of this series using the reset test
> source that can be simulated via sysfs. Specifically I ran this test:
>
> for i in $(seq 1000); do
> echo "LOOP $i --------"
> echo 1 > /sys/kernel/debug/mwifiex/mlan0/reset
>
> while true; do
> if ! ping -w15 -c1 "${GW}" >/dev/null 2>&1; then
> fail=$(( fail + 1 ))
> echo "Fail WiFi ${fail}"
> if [[ ${fail} == 3 ]]; then
> exit 1
> fi
> else
> fail=0
> break
> fi
> done
>
> hciconfig hci0 down
> sleep 1
> if ! hciconfig hci0 up; then
> echo "Fail BT"
> exit 1
> fi
> done
>
> I ran this several times and got several hundred iterations each
> before a failure. When I saw failures:
>
> * Once I saw a "Fail BT"; manually resetting the card again fixed it.
> I didn't give it time to see if it would have detected this
> automatically.
> * Once I saw the ping fail because (for some reason) my device only
> got an IPv6 address from my router and the IPv4 ping failed. I
> changed my script to use 'ping6' to see if that would help.
> * Once I saw the ping fail because the higher level network stack
> ("shill" in my case) seemed to crash. A few minutes later the
> system recovered itself automatically. https://crbug.com/984593 if
> you want more details.
> * Sometimes while I was testing I saw "Fail WiFi 1" indicating a
> transitory failure. Usually this was an association failure, but in
> one case I saw the device do "Firmware wakeup failed" after I
> triggered the reset. This caused the driver to trigger a re-reset
> of itself which eventually recovered things. This was good because
> it was an actual test of the normal reset flow (not the one
> triggered via sysfs).
>
> Changes in v2:
> - s/routnine/routine (Brian Norris, Matthias Kaehlcke).
> - s/contining/containing (Matthias Kaehlcke).
> - Add Matthias Reviewed-by tag.
> - Removed clear_bit() calls and old comment (Brian Norris).
> - Explicit CC of Andreas Fenkart.
> - Explicit CC of Brian Norris.
> - Add "Fixes" pointing at the commit Brian talked about.
> - Add Brian's Reviewed-by tag.
>
> Douglas Anderson (2):
> mmc: core: Add sdio_trigger_replug() API
> mwifiex: Make use of the new sdio_trigger_replug() API to reset
>
> drivers/mmc/core/core.c | 28 +++++++++++++++++++--
> drivers/mmc/core/sdio_io.c | 20 +++++++++++++++
> drivers/net/wireless/marvell/mwifiex/sdio.c | 16 +-----------
> include/linux/mmc/host.h | 15 ++++++++++-
> include/linux/mmc/sdio_func.h | 2 ++
> 5 files changed, 63 insertions(+), 18 deletions(-)
>
Doug, thanks for sending this!
As you know, I have been working on additional changes for SDIO
suspend/resume (still WIP and not ready for sharing) and this series
is related.
The thing is, that even during system suspend/resume, synchronizations
are needed between the different layers (mmc host, mmc core and
sdio-funcs), which is common to the problem you want to solve.
That said, I need to scratch my head a bit more before I can provide
you some feedback on $subject series. Moreover, it's vacation period
at my side so things are moving a bit slower. Please be patient.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v4 net-next 13/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-07-25 13:28 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem
In-Reply-To: <20190722214023.9513-14-snelson@pensando.io>
On Mon, Jul 22, 2019 at 02:40:17PM -0700, Shannon Nelson wrote:
> +static void ionic_get_pauseparam(struct net_device *netdev,
> + struct ethtool_pauseparam *pause)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + struct ionic_dev *idev = &lif->ionic->idev;
> + uint8_t pause_type = idev->port_info->config.pause_type;
> +
> + pause->autoneg = 0;
> +
> + if (pause_type) {
> + pause->rx_pause = pause_type & IONIC_PAUSE_F_RX ? 1 : 0;
> + pause->tx_pause = pause_type & IONIC_PAUSE_F_TX ? 1 : 0;
> + }
> +}
> +
> +static int ionic_set_pauseparam(struct net_device *netdev,
> + struct ethtool_pauseparam *pause)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + struct ionic *ionic = lif->ionic;
> + struct ionic_dev *idev = &lif->ionic->idev;
> + u32 requested_pause;
> + int err;
> +
> + if (pause->autoneg == AUTONEG_ENABLE) {
> + netdev_info(netdev, "Please use 'ethtool -s ...' to change autoneg\n");
> + return -EOPNOTSUPP;
> + }
> +
> + /* change both at the same time */
> + requested_pause = PORT_PAUSE_TYPE_LINK;
> + if (pause->rx_pause)
> + requested_pause |= IONIC_PAUSE_F_RX;
> + if (pause->tx_pause)
> + requested_pause |= IONIC_PAUSE_F_TX;
Hi Shannon
This not what i was expecting, and i'm guessing what you have here is
not right.
So you don't support the case of pause->autoneg ==
AUTONEG_ENABLE. That implies that setting pause values directly
configures the MAC. The values from auto-neg are always ignored. Then
why did you set PAUSE in the get ksettings function?
If you always ignore the values from auto-neg, then your info message
also makes no sense.
What really does the firmware do?
Andrew
^ permalink raw reply
* RE: [PATCH v6 0/5] net: macb: cover letter
From: Parshuram Raju Thombare @ 2019-07-25 13:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
f.fainelli@gmail.com, linux@armlinux.org.uk,
netdev@vger.kernel.org, hkallweit1@gmail.com,
linux-kernel@vger.kernel.org, Rafal Ciepiela, Piotr Sroka,
Anil Joy Varughese, Arthur Marris, Steven Ho, Milind Parab
In-Reply-To: <20190718151310.GE25635@lunn.ch>
Hi Andrew,
>One thing which was never clear is how you are testing the features you are
>adding. Please could you describe your test setup and how each new feature
>is tested using that hardware. I'm particularly interested in what C45 device
>are you using? But i expect Russell would like to know more about SFP
>modules you are using. Do you have any which require 1000BaseX,
>2500BaseX, or provide copper 1G?
Sorry for late reply.
Here is a little more information on our setup used for testing C45 patch with a view to
try clarify a few points.
Regarding the MDIO communication channel that our controller supports - We have tested
MDIO transfers through Clause 22, but none of our local PHY's support Clause 45 so our hardware
team have created an example Clause 45 slave device for us to add support to the driver.
Note our hardware has been in silicon for 20 years, with customers using their own software to support
MDIO (both clause 22 and clause 45 functionality) and so this has been in Cadence's hardware controller
many times.
The programming interface is not hugely different between the two clauses and therefore we feel the risk is low.
For other features like SGMII, USXGMII we are using kc705 and vcu118 FPGA boards.
10G SFP+ module from Tyco electronics is used for testing 10G USXGMII in fixed AN mode.
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-25 13:26 UTC (permalink / raw)
To: Jon Hunter, Jose Abreu, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai,
Robin Murphy, wens@csie.org, linux-tegra, peppe.cavallaro@st.com
In-Reply-To: <7a79be5d-7ba2-c457-36d3-1ccef6572181@nvidia.com>
From: Jon Hunter <jonathanh@nvidia.com>
Date: Jul/25/2019, 14:20:07 (UTC+00:00)
>
> On 03/07/2019 11:37, Jose Abreu wrote:
> > Mapping and unmapping DMA region is an high bottleneck in stmmac driver,
> > specially in the RX path.
> >
> > This commit introduces support for Page Pool API and uses it in all RX
> > queues. With this change, we get more stable troughput and some increase
> > of banwidth with iperf:
> > - MAC1000 - 950 Mbps
> > - XGMAC: 9.22 Gbps
> >
> > Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> > Cc: Joao Pinto <jpinto@synopsys.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> > Cc: Alexandre Torgue <alexandre.torgue@st.com>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
> > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 10 +-
> > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 196 ++++++----------------
> > 3 files changed, 63 insertions(+), 144 deletions(-)
>
> ...
>
> > @@ -3306,49 +3295,22 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
> > else
> > p = rx_q->dma_rx + entry;
> >
> > - if (likely(!rx_q->rx_skbuff[entry])) {
> > - struct sk_buff *skb;
> > -
> > - skb = netdev_alloc_skb_ip_align(priv->dev, bfsize);
> > - if (unlikely(!skb)) {
> > - /* so for a while no zero-copy! */
> > - rx_q->rx_zeroc_thresh = STMMAC_RX_THRESH;
> > - if (unlikely(net_ratelimit()))
> > - dev_err(priv->device,
> > - "fail to alloc skb entry %d\n",
> > - entry);
> > - break;
> > - }
> > -
> > - rx_q->rx_skbuff[entry] = skb;
> > - rx_q->rx_skbuff_dma[entry] =
> > - dma_map_single(priv->device, skb->data, bfsize,
> > - DMA_FROM_DEVICE);
> > - if (dma_mapping_error(priv->device,
> > - rx_q->rx_skbuff_dma[entry])) {
> > - netdev_err(priv->dev, "Rx DMA map failed\n");
> > - dev_kfree_skb(skb);
> > + if (!buf->page) {
> > + buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
> > + if (!buf->page)
> > break;
> > - }
> > -
> > - stmmac_set_desc_addr(priv, p, rx_q->rx_skbuff_dma[entry]);
> > - stmmac_refill_desc3(priv, rx_q, p);
> > -
> > - if (rx_q->rx_zeroc_thresh > 0)
> > - rx_q->rx_zeroc_thresh--;
> > -
> > - netif_dbg(priv, rx_status, priv->dev,
> > - "refill entry #%d\n", entry);
> > }
> > - dma_wmb();
> > +
> > + buf->addr = buf->page->dma_addr;
> > + stmmac_set_desc_addr(priv, p, buf->addr);
> > + stmmac_refill_desc3(priv, rx_q, p);
> >
> > rx_q->rx_count_frames++;
> > rx_q->rx_count_frames %= priv->rx_coal_frames;
> > use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
> >
> > - stmmac_set_rx_owner(priv, p, use_rx_wd);
> > -
> > dma_wmb();
> > + stmmac_set_rx_owner(priv, p, use_rx_wd);
> >
> > entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
> > }
>
> I was looking at this change in a bit closer detail and one thing that
> stuck out to me was the above where the barrier had been moved from
> after the stmmac_set_rx_owner() call to before.
>
> So I moved this back and I no longer saw the crash. However, then I
> recalled I had the patch to enable the debug prints for the buffer
> address applied but after reverting that, the crash occurred again.
>
> In other words, what works for me is moving the above barrier and adding
> the debug print, which appears to suggest that there is some
> timing/coherency issue here. Anyway, maybe this is clue to what is going
> on?
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a7486c2f3221..2f016397231b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3303,8 +3303,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
> rx_q->rx_count_frames %= priv->rx_coal_frames;
> use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
>
> - dma_wmb();
> stmmac_set_rx_owner(priv, p, use_rx_wd);
> + dma_wmb();
>
> entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
> }
> @@ -3438,6 +3438,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
> dma_sync_single_for_device(priv->device, buf->addr,
> frame_len, DMA_FROM_DEVICE);
>
> + pr_info("%s: paddr=0x%llx, vaddr=0x%llx, len=%d", __func__,
> + buf->addr, page_address(buf->page),
> + frame_len);
> +
> if (netif_msg_pktdata(priv)) {
> netdev_dbg(priv->dev, "frame received (%dbytes)",
> frame_len);
>
> Cheers
> Jon
>
> --
> nvpublic
Well, I wasn't expecting that :/
Per documentation of barriers I think we should set descriptor fields
and then barrier and finally ownership to HW so that remaining fields
are coherent before owner is set.
Anyway, can you also add a dma_rmb() after the call to
stmmac_rx_status() ?
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-25 13:21 UTC (permalink / raw)
To: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel,
allan.nielsen
In-Reply-To: <7e7a7015-6072-d884-b2ba-0a51177245ab@cumulusnetworks.com>
On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
> On 25/07/2019 14:44, Horatiu Vultur wrote:
>> There is no way to configure the bridge, to receive only specific link
>> layer multicast addresses. From the description of the command 'bridge
>> fdb append' is supposed to do that, but there was no way to notify the
>> network driver that the bridge joined a group, because LLADDR was added
>> to the unicast netdev_hw_addr_list.
>>
>> Therefore update fdb_add_entry to check if the NLM_F_APPEND flag is set
>> and if the source is NULL, which represent the bridge itself. Then add
>> address to multicast netdev_hw_addr_list for each bridge interfaces.
>> And then the .ndo_set_rx_mode function on the driver is called. To notify
>> the driver that the list of multicast mac addresses changed.
>>
>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>> ---
>> net/bridge/br_fdb.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 46 insertions(+), 3 deletions(-)
>>
>
> Hi,
> I'm sorry but this patch is wrong on many levels, some notes below. In general
> NLM_F_APPEND is only used in vxlan, the bridge does not handle that flag at all.
> FDB is only for *unicast*, nothing is joined and no multicast should be used with fdbs.
> MDB is used for multicast handling, but both of these are used for forwarding.
> The reason the static fdbs are added to the filter is for non-promisc ports, so they can
> receive traffic destined for these FDBs for forwarding.
> If you'd like to join any multicast group please use the standard way, if you'd like to join
> it only on a specific port - join it only on that port (or ports) and the bridge and you'll
And obviously this is for the case where you're not enabling port promisc mode (non-default).
In general you'll only need to join the group on the bridge to receive traffic for it
or add it as an mdb entry to forward it.
> have the effect that you're describing. What do you mean there's no way ?
>
> In addition you're allowing a mix of mcast functions to be called with unicast addresses
> and vice versa, it is not that big of a deal because the kernel will simply return an error
> but still makes no sense.
>
> Nacked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index b1d3248..d93746d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -175,6 +175,29 @@ static void fdb_add_hw_addr(struct net_bridge *br, const unsigned char *addr)
>> }
>> }
>>
>> +static void fdb_add_hw_maddr(struct net_bridge *br, const unsigned char *addr)
>> +{
>> + int err;
>> + struct net_bridge_port *p;
>> +
>> + ASSERT_RTNL();
>> +
>> + list_for_each_entry(p, &br->port_list, list) {
>> + if (!br_promisc_port(p)) {
>> + err = dev_mc_add(p->dev, addr);
>> + if (err)
>> + goto undo;
>> + }
>> + }
>> +
>> + return;
>> +undo:
>> + list_for_each_entry_continue_reverse(p, &br->port_list, list) {
>> + if (!br_promisc_port(p))
>> + dev_mc_del(p->dev, addr);
>> + }
>> +}
>> +
>> /* When a static FDB entry is deleted, the HW address from that entry is
>> * also removed from the bridge private HW address list and updates all
>> * the ports with needed information.
>> @@ -192,13 +215,27 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
>> }
>> }
>>
>> +static void fdb_del_hw_maddr(struct net_bridge *br, const unsigned char *addr)
>> +{
>> + struct net_bridge_port *p;
>> +
>> + ASSERT_RTNL();
>> +
>> + list_for_each_entry(p, &br->port_list, list) {
>> + if (!br_promisc_port(p))
>> + dev_mc_del(p->dev, addr);
>> + }
>> +}
>> +
>> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
>> bool swdev_notify)
>> {
>> trace_fdb_delete(br, f);
>>
>> - if (f->is_static)
>> + if (f->is_static) {
>> fdb_del_hw_addr(br, f->key.addr.addr);
>> + fdb_del_hw_maddr(br, f->key.addr.addr);
>
> Walking over all ports again for each static delete is a no-go.
>
>> + }
>>
>> hlist_del_init_rcu(&f->fdb_node);
>> rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode,
>> @@ -843,13 +880,19 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> fdb->is_local = 1;
>> if (!fdb->is_static) {
>> fdb->is_static = 1;
>> - fdb_add_hw_addr(br, addr);
>> + if (flags & NLM_F_APPEND && !source)
>> + fdb_add_hw_maddr(br, addr);
>> + else
>> + fdb_add_hw_addr(br, addr);
>> }
>> } else if (state & NUD_NOARP) {
>> fdb->is_local = 0;
>> if (!fdb->is_static) {
>> fdb->is_static = 1;
>> - fdb_add_hw_addr(br, addr);
>> + if (flags & NLM_F_APPEND && !source)
>> + fdb_add_hw_maddr(br, addr);
>> + else
>> + fdb_add_hw_addr(br, addr);
>> }
>> } else {
>> fdb->is_local = 0;
>>
>
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jon Hunter @ 2019-07-25 13:20 UTC (permalink / raw)
To: Jose Abreu, linux-kernel, netdev, linux-stm32, linux-arm-kernel
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai,
Robin Murphy, wens@csie.org, linux-tegra, peppe.cavallaro@st.com
In-Reply-To: <1b254bb7fc6044c5e6e2fdd9e00088d1d13a808b.1562149883.git.joabreu@synopsys.com>
On 03/07/2019 11:37, Jose Abreu wrote:
> Mapping and unmapping DMA region is an high bottleneck in stmmac driver,
> specially in the RX path.
>
> This commit introduces support for Page Pool API and uses it in all RX
> queues. With this change, we get more stable troughput and some increase
> of banwidth with iperf:
> - MAC1000 - 950 Mbps
> - XGMAC: 9.22 Gbps
>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 10 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 196 ++++++----------------
> 3 files changed, 63 insertions(+), 144 deletions(-)
...
> @@ -3306,49 +3295,22 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
> else
> p = rx_q->dma_rx + entry;
>
> - if (likely(!rx_q->rx_skbuff[entry])) {
> - struct sk_buff *skb;
> -
> - skb = netdev_alloc_skb_ip_align(priv->dev, bfsize);
> - if (unlikely(!skb)) {
> - /* so for a while no zero-copy! */
> - rx_q->rx_zeroc_thresh = STMMAC_RX_THRESH;
> - if (unlikely(net_ratelimit()))
> - dev_err(priv->device,
> - "fail to alloc skb entry %d\n",
> - entry);
> - break;
> - }
> -
> - rx_q->rx_skbuff[entry] = skb;
> - rx_q->rx_skbuff_dma[entry] =
> - dma_map_single(priv->device, skb->data, bfsize,
> - DMA_FROM_DEVICE);
> - if (dma_mapping_error(priv->device,
> - rx_q->rx_skbuff_dma[entry])) {
> - netdev_err(priv->dev, "Rx DMA map failed\n");
> - dev_kfree_skb(skb);
> + if (!buf->page) {
> + buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
> + if (!buf->page)
> break;
> - }
> -
> - stmmac_set_desc_addr(priv, p, rx_q->rx_skbuff_dma[entry]);
> - stmmac_refill_desc3(priv, rx_q, p);
> -
> - if (rx_q->rx_zeroc_thresh > 0)
> - rx_q->rx_zeroc_thresh--;
> -
> - netif_dbg(priv, rx_status, priv->dev,
> - "refill entry #%d\n", entry);
> }
> - dma_wmb();
> +
> + buf->addr = buf->page->dma_addr;
> + stmmac_set_desc_addr(priv, p, buf->addr);
> + stmmac_refill_desc3(priv, rx_q, p);
>
> rx_q->rx_count_frames++;
> rx_q->rx_count_frames %= priv->rx_coal_frames;
> use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
>
> - stmmac_set_rx_owner(priv, p, use_rx_wd);
> -
> dma_wmb();
> + stmmac_set_rx_owner(priv, p, use_rx_wd);
>
> entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
> }
I was looking at this change in a bit closer detail and one thing that
stuck out to me was the above where the barrier had been moved from
after the stmmac_set_rx_owner() call to before.
So I moved this back and I no longer saw the crash. However, then I
recalled I had the patch to enable the debug prints for the buffer
address applied but after reverting that, the crash occurred again.
In other words, what works for me is moving the above barrier and adding
the debug print, which appears to suggest that there is some
timing/coherency issue here. Anyway, maybe this is clue to what is going
on?
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a7486c2f3221..2f016397231b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3303,8 +3303,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
rx_q->rx_count_frames %= priv->rx_coal_frames;
use_rx_wd = priv->use_riwt && rx_q->rx_count_frames;
- dma_wmb();
stmmac_set_rx_owner(priv, p, use_rx_wd);
+ dma_wmb();
entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}
@@ -3438,6 +3438,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
dma_sync_single_for_device(priv->device, buf->addr,
frame_len, DMA_FROM_DEVICE);
+ pr_info("%s: paddr=0x%llx, vaddr=0x%llx, len=%d", __func__,
+ buf->addr, page_address(buf->page),
+ frame_len);
+
if (netif_msg_pktdata(priv)) {
netdev_dbg(priv->dev, "frame received (%dbytes)",
frame_len);
Cheers
Jon
--
nvpublic
^ permalink raw reply related
* Re: [PATCH net] net: hns: fix LED configuration for marvell phy
From: Andrew Lunn @ 2019-07-25 13:08 UTC (permalink / raw)
To: liuyonglong
Cc: David Miller, netdev, linux-kernel, linuxarm, salil.mehta,
yisen.zhuang, shiju.jose
In-Reply-To: <8017d9ff-2991-f94f-e611-4d1bac12e93b@huawei.com>
> You are discussing about the DT configuration, is Matthias Kaehlcke's work
> also provide a generic way to configure PHY LEDS using ACPI?
In general, you should be able to use the same properties in ACPI as
DT. If the device_property_read_X() API is used, it will try both ACPI
and OF to get the property.
Andrew
^ permalink raw reply
* Re: [PATCH][next] can: kvaser_pciefd: remove redundant negative check on trigger
From: Colin Ian King @ 2019-07-25 12:41 UTC (permalink / raw)
To: wharms
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller,
linux-can, netdev, kernel-janitors, linux-kernel
In-Reply-To: <5D39A274.1000800@bfs.de>
On 25/07/2019 13:37, walter harms wrote:
>
>
> Am 25.07.2019 13:25, schrieb Colin King:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The check to see if trigger is less than zero is always false, trigger
>> is always in the range 0..255. Hence the check is redundant and can
>> be removed.
>>
>> Addresses-Coverity: ("Logically dead code")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/can/kvaser_pciefd.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
>> index 3af747cbbde4..68e00aad0810 100644
>> --- a/drivers/net/can/kvaser_pciefd.c
>> +++ b/drivers/net/can/kvaser_pciefd.c
>> @@ -652,9 +652,6 @@ static void kvaser_pciefd_pwm_stop(struct kvaser_pciefd_can *can)
>> top = (pwm_ctrl >> KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT) & 0xff;
>>
>> trigger = (100 * top + 50) / 100;
>> - if (trigger < 0)
>> - trigger = 0;
>> -
> to be fair i do not understand the calculation here here.
> (100*top+50)/100 = top+50/100
>
> but seems to be int so it should be =top
Indeed it does not do anything, that does look like an unintentional
bug. Good catch.
>
> did i missunderstand something here ?
>
> re,
> wh
>
>
>> pwm_ctrl = trigger & 0xff;
>> pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
>> iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
^ permalink raw reply
* Re: [PATCH] net: sfc: falcon: convert to i2c_new_dummy_device
From: Edward Cree @ 2019-07-25 12:37 UTC (permalink / raw)
To: David Miller, wsa+renesas
Cc: linux-i2c, linux-net-drivers, mhabets, netdev, linux-kernel
In-Reply-To: <20190724.154739.72147269285837223.davem@davemloft.net>
On 24/07/2019 23:47, David Miller wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Date: Mon, 22 Jul 2019 19:26:35 +0200
>
>> Move from i2c_new_dummy() to i2c_new_dummy_device(). So, we now get an
>> ERRPTR which we use in error handling.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Subject & description are incomplete, you're also changing i2c_new_device()
to i2c_new_client_device().
Other than that,
Acked-by: Edward Cree <ecree@solarflare.com>
> Solarflare folks, please review/test.
>
> Thank you.
Falcon isn't likely to get tested by us, I think we only have about three
of them left in the building, two of which are in display cabinets ;-)
We end-of-lifed this hardware a couple of years ago, maybe it should be
downgraded from 'supported' to 'odd fixes'. Or even moved to staging,
like that qlogic driver recently was.
^ permalink raw reply
* Re: [PATCH][next] can: kvaser_pciefd: remove redundant negative check on trigger
From: walter harms @ 2019-07-25 12:37 UTC (permalink / raw)
To: Colin King
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller,
linux-can, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190725112509.1075-1-colin.king@canonical.com>
Am 25.07.2019 13:25, schrieb Colin King:
> From: Colin Ian King <colin.king@canonical.com>
>
> The check to see if trigger is less than zero is always false, trigger
> is always in the range 0..255. Hence the check is redundant and can
> be removed.
>
> Addresses-Coverity: ("Logically dead code")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/can/kvaser_pciefd.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
> index 3af747cbbde4..68e00aad0810 100644
> --- a/drivers/net/can/kvaser_pciefd.c
> +++ b/drivers/net/can/kvaser_pciefd.c
> @@ -652,9 +652,6 @@ static void kvaser_pciefd_pwm_stop(struct kvaser_pciefd_can *can)
> top = (pwm_ctrl >> KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT) & 0xff;
>
> trigger = (100 * top + 50) / 100;
> - if (trigger < 0)
> - trigger = 0;
> -
to be fair i do not understand the calculation here here.
(100*top+50)/100 = top+50/100
but seems to be int so it should be =top
did i missunderstand something here ?
re,
wh
> pwm_ctrl = trigger & 0xff;
> pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
> iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
^ permalink raw reply
* Re: [PATCH net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: liuyonglong @ 2019-07-25 12:28 UTC (permalink / raw)
To: Saeed Mahameed, tanhuazhong@huawei.com, davem@davemloft.net
Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
salil.mehta@huawei.com, linuxarm@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ffd942e7d7442549a3a6d469709b7f7405928afe.camel@mellanox.com>
On 2019/7/25 3:12, Saeed Mahameed wrote:
> On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
>> From: Yonglong Liu <liuyonglong@huawei.com>
>>
>> Some times just see the eth interface have been down/up via
>> dmesg, but can not know why the eth down. So adds some debug
>> messages to identify the cause for this.
>>
>
> I really don't like this. your default msg lvl has NETIF_MSG_IFDOWN
> turned on .. dumping every single operation that happens on your device
> by default to kernel log is too much !
>
> We should really consider using trace buffers with well defined
> structures for vendor specific events. so we can use bpf filters and
> state of the art tools for netdev debugging.
>
We do this because we can just see a link down message in dmesg, and had
take a long time to found the cause of link down, just because another
user changed the settings.
We can change the net_open/net_stop/dcbnl_ops to msg_drv (not default
turned on), and want to keep the others default print to kernel log,
is it acceptable?
>> @@ -1593,6 +1603,11 @@ static int hns3_ndo_set_vf_vlan(struct
>> net_device *netdev, int vf, u16 vlan,
>> struct hnae3_handle *h = hns3_get_handle(netdev);
>> int ret = -EIO;
>>
>> + if (netif_msg_ifdown(h))
>
> why msg_ifdown ? looks like netif_msg_drv is more appropriate, for many
> of the cases in this patch.
>
This operation may cause link down, so we use msg_ifdown.
>> + netdev_info(netdev,
>> + "set vf vlan: vf=%d, vlan=%d, qos=%d,
>> vlan_proto=%d\n",
>> + vf, vlan, qos, vlan_proto);
>> +
>> if (h->ae_algo->ops->set_vf_vlan_filter)
>> ret = h->ae_algo->ops->set_vf_vlan_filter(h, vf, vlan,
>> qos,
>> vlan_proto);
>> @@ -1611,6 +1626,10 @@ static int hns3_nic_change_mtu(struct
>> net_device *netdev, int new_mtu)
>> if (!h->ae_algo->ops->set_mtu)
>> return -EOPNOTSUPP;
>>
>> + if (netif_msg_ifdown(h))
>> + netdev_info(netdev, "change mtu from %d to %d\n",
>> + netdev->mtu, new_mtu);
>> +
>> ret = h->ae_algo->ops->set_mtu(h, new_mtu);
>> if (ret)
>> netdev_err(netdev, "failed to change MTU in hardware
>> %d\n",
>> @@ -4395,6 +4414,11 @@ int hns3_set_channels(struct net_device
>> *netdev,
>> if (kinfo->rss_size == new_tqp_num)
>> return 0;
>>
>> + if (netif_msg_ifdown(h))
>> + netdev_info(netdev,
>> + "set channels: tqp_num=%d, rxfh=%d\n",
>> + new_tqp_num, rxfh_configured);
>> +
>> ret = hns3_reset_notify(h, HNAE3_DOWN_CLIENT);
>> if (ret)
>> return ret;
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> index e71c92b..edb9845 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
>> @@ -311,6 +311,9 @@ static void hns3_self_test(struct net_device
>> *ndev,
>> if (eth_test->flags != ETH_TEST_FL_OFFLINE)
>> return;
>>
>> + if (netif_msg_ifdown(h))
>> + netdev_info(ndev, "self test start\n");
>> +
>> st_param[HNAE3_LOOP_APP][0] = HNAE3_LOOP_APP;
>> st_param[HNAE3_LOOP_APP][1] =
>> h->flags & HNAE3_SUPPORT_APP_LOOPBACK;
>> @@ -374,6 +377,9 @@ static void hns3_self_test(struct net_device
>> *ndev,
>>
>> if (if_running)
>> ndev->netdev_ops->ndo_open(ndev);
>> +
>> + if (netif_msg_ifdown(h))
>> + netdev_info(ndev, "self test end\n");
>> }
>>
>> static int hns3_get_sset_count(struct net_device *netdev, int
>> stringset)
>> @@ -604,6 +610,11 @@ static int hns3_set_pauseparam(struct net_device
>> *netdev,
>> {
>> struct hnae3_handle *h = hns3_get_handle(netdev);
>>
>> + if (netif_msg_ifdown(h))
>> + netdev_info(netdev,
>> + "set pauseparam: autoneg=%d, rx:%d,
>> tx:%d\n",
>> + param->autoneg, param->rx_pause, param-
>>> tx_pause);
>> +
>> if (h->ae_algo->ops->set_pauseparam)
>> return h->ae_algo->ops->set_pauseparam(h, param-
>>> autoneg,
>> param->rx_pause,
>> @@ -743,6 +754,13 @@ static int hns3_set_link_ksettings(struct
>> net_device *netdev,
>> if (cmd->base.speed == SPEED_1000 && cmd->base.duplex ==
>> DUPLEX_HALF)
>> return -EINVAL;
>>
>> + if (netif_msg_ifdown(handle))
>> + netdev_info(netdev,
>> + "set link(%s): autoneg=%d, speed=%d,
>> duplex=%d\n",
>> + netdev->phydev ? "phy" : "mac",
>> + cmd->base.autoneg, cmd->base.speed,
>> + cmd->base.duplex);
>> +
>> /* Only support ksettings_set for netdev with phy attached for
>> now */
>> if (netdev->phydev)
>> return phy_ethtool_ksettings_set(netdev->phydev, cmd);
>> @@ -984,6 +1002,10 @@ static int hns3_nway_reset(struct net_device
>> *netdev)
>> return -EINVAL;
>> }
>>
>> + if (netif_msg_ifdown(handle))
>> + netdev_info(netdev, "nway reset (using %s)\n",
>> + phy ? "phy" : "mac");
>> +
>> if (phy)
>> return genphy_restart_aneg(phy);
>>
>> @@ -1308,6 +1330,10 @@ static int hns3_set_fecparam(struct net_device
>> *netdev,
>> if (!ops->set_fec)
>> return -EOPNOTSUPP;
>> fec_mode = eth_to_loc_fec(fec->fec);
>> +
>> + if (netif_msg_ifdown(handle))
>> + netdev_info(netdev, "set fecparam: mode=%d\n",
>> fec_mode);
>> +
>> return ops->set_fec(handle, fec_mode);
>> }
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
>> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
>> index bac4ce1..133e7c6 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
>> @@ -201,6 +201,7 @@ static int hclge_client_setup_tc(struct hclge_dev
>> *hdev)
>> static int hclge_ieee_setets(struct hnae3_handle *h, struct ieee_ets
>> *ets)
>> {
>> struct hclge_vport *vport = hclge_get_vport(h);
>> + struct net_device *netdev = h->kinfo.netdev;
>> struct hclge_dev *hdev = vport->back;
>> bool map_changed = false;
>> u8 num_tc = 0;
>> @@ -215,6 +216,9 @@ static int hclge_ieee_setets(struct hnae3_handle
>> *h, struct ieee_ets *ets)
>> return ret;
>>
>> if (map_changed) {
>> + if (netif_msg_ifdown(h))
>> + netdev_info(netdev, "set ets\n");
>> +
>> ret = hclge_notify_client(hdev, HNAE3_DOWN_CLIENT);
>> if (ret)
>> return ret;
>> @@ -300,6 +304,7 @@ static int hclge_ieee_getpfc(struct hnae3_handle
>> *h, struct ieee_pfc *pfc)
>> static int hclge_ieee_setpfc(struct hnae3_handle *h, struct ieee_pfc
>> *pfc)
>> {
>> struct hclge_vport *vport = hclge_get_vport(h);
>> + struct net_device *netdev = h->kinfo.netdev;
>> struct hclge_dev *hdev = vport->back;
>> u8 i, j, pfc_map, *prio_tc;
>>
>> @@ -325,6 +330,11 @@ static int hclge_ieee_setpfc(struct hnae3_handle
>> *h, struct ieee_pfc *pfc)
>> hdev->tm_info.hw_pfc_map = pfc_map;
>> hdev->tm_info.pfc_en = pfc->pfc_en;
>>
>> + if (netif_msg_ifdown(h))
>> + netdev_info(netdev,
>> + "set pfc: pfc_en=%d, pfc_map=%d,
>> num_tc=%d\n",
>> + pfc->pfc_en, pfc_map, hdev-
>>> tm_info.num_tc);
>> +
>> hclge_tm_pfc_info_update(hdev);
>>
>> return hclge_pause_setup_hw(hdev, false);
>> @@ -345,8 +355,12 @@ static u8 hclge_getdcbx(struct hnae3_handle *h)
>> static u8 hclge_setdcbx(struct hnae3_handle *h, u8 mode)
>> {
>> struct hclge_vport *vport = hclge_get_vport(h);
>> + struct net_device *netdev = h->kinfo.netdev;
>> struct hclge_dev *hdev = vport->back;
>>
>> + if (netif_msg_drv(h))
>> + netdev_info(netdev, "set dcbx: mode=%d\n", mode);
>> +
>> /* No support for LLD_MANAGED modes or CEE */
>> if ((mode & DCB_CAP_DCBX_LLD_MANAGED) ||
>> (mode & DCB_CAP_DCBX_VER_CEE) ||
^ permalink raw reply
* Re: [PATCH net-next 2/2] mlx4/en_netdev: call notifiers when hw_enc_features change
From: Davide Caratti @ 2019-07-25 12:25 UTC (permalink / raw)
To: Saeed Mahameed, davem@davemloft.net, Tariq Toukan,
netdev@vger.kernel.org
Cc: Eran Ben Elisha
In-Reply-To: <e007bac4c951486294d4e69d20f7c9ed7040172d.camel@mellanox.com>
On Wed, 2019-07-24 at 20:47 +0000, Saeed Mahameed wrote:
> On Wed, 2019-07-24 at 16:02 +0200, Davide Caratti wrote:
> > ensure to call netdev_features_change() when the driver flips its
> > hw_enc_features bits.
> >
> > Signed-off-by: Davide Caratti <dcaratti@redhat.com>
>
> The patch is correct,
hello Saeed, and thanks for looking at this!
> but can you explain how did you come to this ?
> did you encounter any issue with the current code ?
>
> I am asking just because i think the whole dynamic changing of dev-
> > hw_enc_features is redundant since mlx4 has the featutres_check
> callback.
we need it to ensure that vlan_transfer_features() updates
the (new) value of hw_enc_features in the overlying vlan: otherwise,
segmentation will happen anyway when skb passes from vxlan to vlan, if the
vxlan is added after the vlan device has been created (see: 7dad9937e064
("net: vlan: add support for tunnel offload") ).
thanks!
--
davide
^ 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