* Re: [PATCH iproute2 net-next] gre6: add collect metadata support
From: William Tu @ 2017-12-04 20:40 UTC (permalink / raw)
To: Gregory Rose; +Cc: Linux Kernel Network Developers
In-Reply-To: <277df688-0a71-bd3f-eb67-c706113bebd2@gmail.com>
On Mon, Dec 4, 2017 at 9:26 AM, Gregory Rose <gvrose8192@gmail.com> wrote:
> On 12/1/2017 3:03 PM, William Tu wrote:
>>
>> The patch adds 'external' option to support collect metadata
>> gre6 tunnel. Example of L3 and L2 gre device:
>> bash:~# ip link add dev ip6gre123 type ip6gre external
>> bash:~# ip link add dev ip6gretap123 type ip6gretap external
>
>
> Hi William,
>
> one question. Is there ever actually going to be a situation in which an
> ipv6 gre tunnel will go from
> external to not external? I'm wondering if the "no external" option is
> really needed since default is
> no and I'm not sure under what circumstances a tunnel would go from external
> to not external.
>
> Thanks,
>
> - Greg
good point. I don't think we should support setting a tunnel from
external to no external at run-time.
Let me remove the "noexternal" description in man page.
Thanks
William
^ permalink raw reply
* Re: [PATCH] tcp/dccp: CVE-2017-8824: use-after-free in DCCP code
From: Eric Dumazet @ 2017-12-04 20:48 UTC (permalink / raw)
To: simo.ghannam, netdev
In-Reply-To: <1512420060.19682.62.camel@gmail.com>
On Mon, 2017-12-04 at 12:41 -0800, Eric Dumazet wrote:
> On Mon, 2017-12-04 at 21:27 +0100, simo.ghannam@gmail.com wrote:
> > From: Mohamed Ghannam <simo.ghannam@gmail.com>
> >
> > Whenever the sock object is in DCCP_CLOSED state,
> > dccp_disconnect() must free
> > dccps_hc_tx_ccid and dccps_hc_rx_ccid and set to NULL.
> >
> > Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
> > ---
>
> Small nit : white spaces in changelog where not needed :
>
> Whenever the sock object is in DCCP_CLOSED state, dccp_disconnect()
> must free dccps_hc_tx_ccid and dccps_hc_rx_ccid and set them to NULL.
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> Thanks !
>
Also patch title mentions tcp, but this is not a tcp fix, only dccp
[PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
^ permalink raw reply
* Re: Linux network is damn fast, need more use XDP (Was: DC behaviors today)
From: Joel Wirāmu Pauling @ 2017-12-04 20:49 UTC (permalink / raw)
To: Dave Taht
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Christina Jacob,
cerowrt-devel-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1@public.gmane.org,
bloat, David Ahern, Tariq Toukan
In-Reply-To: <CAA93jw4yOz2KoJGz4t9KqFrr=Zx+=N_r-c_W9iQCpGCBCgDVgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 5 December 2017 at 06:00, Dave Taht <dave.taht@gmail.com> wrote:
>>> The route table lookup also really expensive on the main cpu.
>
> To clarify the context here, I was asking specifically if the X5 mellonox card
> did routing table offlload or only switching.
>
To clarify what I know the X5 using it's smart offload engine CAN do
L3 offload into the NIC - the X4's can't.
So for the Nuage OVS -> Eswitch (what mellanox calls the flow
programming) magic to happen and be useful we are going to need X5.
Mark Iskra gave a talk at Openstack summit which can be found here:
https://www.openstack.org/videos/sydney-2017/warp-speed-openvswitch-turbo-charge-vnfs-to-100gbps-in-nextgen-sdnnfv-datacenter
Slides here:
https://www.openstack.org/assets/presentation-media/OSS-Nov-2017-Warp-speed-Openvswitch-v6.pptx
Mark's local to you (Mountain View) - and is a nice guy, is probably
the better person to answer specifics.
-Joel
_______________________________________________
Bloat mailing list
Bloat@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/bloat
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO_HW
From: Alexander Duyck @ 2017-12-04 20:58 UTC (permalink / raw)
To: Michael Chan; +Cc: David Miller, Netdev, Ariel Elior, everest-linux-l2
In-Reply-To: <CACKFLikCBZ3R6VeCUBkA9cy8adySnrEA1t=GS_LbXgX6eE6uOg@mail.gmail.com>
On Mon, Dec 4, 2017 at 11:52 AM, Michael Chan <michael.chan@broadcom.com> wrote:
> On Mon, Dec 4, 2017 at 10:43 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, Dec 4, 2017 at 10:23 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>>> On Mon, Dec 4, 2017 at 8:47 AM, Alexander Duyck
>>> <alexander.duyck@gmail.com> wrote:
>>>> On Mon, Dec 4, 2017 at 3:12 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>>>>> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
>>>>> GRO. With this flag, we can now independently turn on or off hardware
>>>>> GRO when GRO is on. Hardware GRO guarantees that packets can be
>>>>> re-segmented by TSO/GSO to reconstruct the original packet stream.
>>>>>
>>>>> Cc: Ariel Elior <Ariel.Elior@cavium.com>
>>>>> Cc: everest-linux-l2@cavium.com
>>>>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>>>>
>>>> Do we really need yet another feature bit for this? We already have
>>>> LRO and GRO and now we have to add something that isn't quite either
>>>> one?
>>>
>>> I think so, to be consistent with TSO/GSO on the transmit side. On
>>> the receive side, we have LRO/GRO_HW/GRO. There is difference between
>>> LRO/GRO_HW that we need to distinguish between the 2.
>>
>> I don't really see the difference. Your GRO_HW likely doens't do all
>> of the stuff GRO can do. Neither does LRO. Both occur in the hardware
>> normally. It would make sense to reuse the LRO flag for this instead
>> of coming up with a new feature flag that makes things confusing by
>> saying you are doing a software offload in hardware.
>
> I agree it is a little confusing, but LRO + private flag is just as
> confusing in my opinion.
>
>>
>> I view LRO as a subset of what GRO can handle, that is performed in
>> hardware.
>
> I don't view LRO as a subset of GRO. LRO loses information that
> cannot be reconstructed back once an LRO frame is aggregated. LRO is
> just different from GRO. GRO_HW must provide enough information to
> reconstruct the original frames.
I think we can just agree to disagree on this. Defining GRO_HW as
reversible LRO is the way I view it. In my mind LRO is assembly in
hardware, the non-reversible part is an unfortunate side effect of
existing implementations. From the sound of things you and Dave view
the non-reversible part as a core bit of LRO and the hardware/driver
implementation of it is just a secondary thing.
>> From the stack perspective the only thing that really
>> matters is that the frames can be segmented back into what they were
>> before they were assembled. That is why I think it would be better to
>> add a flag indicating that the LRO is reversible instead of adding yet
>> another feature bit that the user has to toggle. That way if at some
>> point in the future an issue is found where your "GRO in hardware"
>> feature has a bug that isn't reversible it is just a matter of
>> clearing the privage flag bit and the mechanisms already in place for
>> dealing with assembly and routing can take over.
>
> NETIF_F_GRO_HW is a flag that depends on NETIF_F_GRO. In some ways,
> it is similar to a private flag that depends on NETIF_F_LRO. But I
> think a standard flag is better.
Why would you make it dependent on NETIF_F_GRO? That doesn't make any
sense to me. It gets in the way of GRO anyway as it can't assemble an
already aggregated frame.
It seems more like the two features should be able to co-exist with
either one being able to be disabled/enabled independently. It makes
it much easier to debug things this way. Otherwise there is no way to
tell if a given issue is software or hardware GRO since disabling
software disables them both.
>>
>>>>
>>>> I think I would rather have something like a netdev private flag that
>>>> says LRO assembled frames are routable and just have this all run over
>>>> the LRO flag with a test for the private flag to avoid triggering the
>>>> LRO disable in the case of the flag being present. Really this is just
>>>> a clean LRO implementation anyway so maybe we should just go that
>>>> route where LRO is the hardware offload and GRO is the generic
>>>> software implementation of that offload. That way when GRO gets some
>>>> new feature that your hardware doesn't support we don't have to argue
>>>> about the differences since LRO is meant to be a limited
>>>> implementation anyway due to the nature of it being in hardware.
>>>
>>> Private flag will work. But a standard feature flag is better since
>>> there are multiple drivers supporting this. A standard way to turn
>>> this on/off is a better user experience. It's also consistent with
>>> TSO/GSO on the transmit side.
>>
>> I agree, and that is why I would prefer to see this use the LRO flag.
>> It is the flag that is normally used to indicating Rx coalescing in
>> hardware. Coming up with a custom feature flag for a form of LRO that
>> your hardware does doesn't make much sense to me. Otherwise I might as
>> well go modify ixgbe and rename the LRO it does to GRO_HW since I can
>> make it do most of what you are doing here.
>
> Again, there is enough difference between LRO and hardware GRO that it
> makes sense to add a new flag. Functionally, a private flag will work
> too, but a standard flag makes more intuitive sense to me and to
> users.
>
> Yeah, the idea is that any vendor can use GRO_HW. Today, there are 3
> drivers supporting it. I'm sure there will be other drivers
> supporting this in the future. For something that is supported by
> multiple vendors, that's another reason to use a standard flag.
>
>>
>>>>
>>>> To me it just seems like this is an attempt to use the GRO name as a
>>>> marketing term and I really don't like the feel of it.
>>>>
>>>
>>> I disagree with this. It's more than a marketing term.
>>
>> Not really. It is a subset of GRO offload in hardware. In my mind that
>> is just LRO. I say subset since odds are you don't support all of the
>> same protocols and tunnels that GRO in software does.
>
> Of course, hardware has some limitations, such as the number of TCP
> connections it can aggregate, etc. But again, it is different from
> LRO.
The bit I don't like about this is that if you bond a device that is
running LRO with one that is running GSO_HW you now have to disable
two flags on the bond in order to disable hardware aggregation.
Admittedly I haven't take a look at the entire patch set, but did you
also take care of the flag sychronization issues this is going to
cause with upper devices such as vlan, macvlan, bond, etc?
^ permalink raw reply
* [net 1/1] tipc: fix memory leak in tipc_accept_from_sock()
From: Jon Maloy @ 2017-12-04 21:00 UTC (permalink / raw)
To: davem, netdev
Cc: tipc-discussion, hoang.h.le, mohan.krishna.ghanta.krishnamurthy
When the function tipc_accept_from_sock() fails to create an instance of
struct tipc_subscriber it omits to free the already created instance of
struct tipc_conn instance before it returns.
We fix that with this commit.
Reported-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/server.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tipc/server.c b/net/tipc/server.c
index acaef80..bcede09 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -314,6 +314,7 @@ static int tipc_accept_from_sock(struct tipc_conn *con)
newcon->usr_data = s->tipc_conn_new(newcon->conid);
if (!newcon->usr_data) {
sock_release(newsock);
+ conn_put(newcon);
return -ENOMEM;
}
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* RE: ethtool-copy.h out of sync with linux header file?
From: Greenwalt, Paul @ 2017-12-04 21:00 UTC (permalink / raw)
To: Scott Branden, linville@tuxdriver.com; +Cc: netdev
In-Reply-To: <e9089cb9-5cc5-b44a-6b49-8523c5323179@broadcom.com>
John,
Can this patch be reverted? As Stephen Hemminger mentioned
there is an ABI compatibility issue with this patch:
https://patchwork.ozlabs.org/patch/806049/#1757846
Thanks,
Paul
> -----Original Message-----
> From: Scott Branden [mailto:scott.branden@broadcom.com]
> Sent: Monday, December 4, 2017 12:07 PM
> To: linville@tuxdriver.com; Greenwalt, Paul <paul.greenwalt@intel.com>
> Cc: netdev <netdev@vger.kernel.org>
> Subject: ethtool-copy.h out of sync with linux header file?
>
> Hi Paul/John,
>
> I see a commit "ethtool: Add DMA Coalescing support" was added to
> ethtool-copy.h yet I cannot find the change in the ethtool.h in the
> linux kernel.
>
> As such, I am unable to commit a change to ethtool-copy.h to sync it
> with net-next. If I sync then ethtool will fail to compile.
>
> Please advise.
>
> Thanks,
>
> Scott
>
>
^ permalink raw reply
* [PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
From: simo.ghannam @ 2017-12-04 21:02 UTC (permalink / raw)
To: netdev; +Cc: Mohamed Ghannam
From: Mohamed Ghannam <simo.ghannam@gmail.com>
Whenever the sock object is in DCCP_CLOSED state, dccp_disconnect() must free
dccps_hc_tx_ccid and dccps_hc_rx_ccid and set to NULL.
Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
---
net/dccp/proto.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index b68168f..9d43c1f 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -259,6 +259,7 @@ int dccp_disconnect(struct sock *sk, int flags)
{
struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_sock *inet = inet_sk(sk);
+ struct dccp_sock *dp = dccp_sk(sk);
int err = 0;
const int old_state = sk->sk_state;
@@ -278,6 +279,10 @@ int dccp_disconnect(struct sock *sk, int flags)
sk->sk_err = ECONNRESET;
dccp_clear_xmit_timers(sk);
+ ccid_hc_rx_delete(dp->dccps_hc_rx_ccid, sk);
+ ccid_hc_tx_delete(dp->dccps_hc_tx_ccid, sk);
+ dp->dccps_hc_rx_ccid = NULL;
+ dp->dccps_hc_tx_ccid = NULL;
__skb_queue_purge(&sk->sk_receive_queue);
__skb_queue_purge(&sk->sk_write_queue);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 2/4] bnxt_en: Use NETIF_F_GRO_HW.
From: Or Gerlitz @ 2017-12-04 21:06 UTC (permalink / raw)
To: Michael Chan; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CACKFLik-yLSCPMyV__CfDZaZfLWCHo6x74-7bWT+Ox1cVJfMpA@mail.gmail.com>
On Mon, Dec 4, 2017 at 8:11 PM, Michael Chan <michael.chan@broadcom.com> wrote:
> All the logic is already in these 3 drivers in the tree. You can see
> the additional logic in any of these drivers. It's just that these
> drivers have been using NETIF_F_GRO to turn on this mode in
> hardware/firmware.
What happens for tcp encaped by some udp tunnel, does
the HW know how to GRO that?
^ permalink raw reply
* [PATCH net-next] rtnetlink: fix rtnl_link msghandler rcu annotations
From: Florian Westphal @ 2017-12-04 21:42 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
Incorrect/missing annotations caused a few sparse warnings:
rtnetlink.c:155:15: incompatible types .. (different address spaces)
rtnetlink.c:157:23: incompatible types .. (different address spaces)
rtnetlink.c:185:15: incompatible types .. (different address spaces)
rtnetlink.c:285:15: incompatible types .. (different address spaces)
rtnetlink.c:317:9: incompatible types .. (different address spaces)
rtnetlink.c:3054:23: incompatible types .. (different address spaces)
no change in generated code.
Fixes: addf9b90de22f7 ("net: rtnetlink: use rcu to free rtnl message handlers")
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 642b3afb12b9..a4faefd65006 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -129,7 +129,7 @@ bool lockdep_rtnl_is_held(void)
EXPORT_SYMBOL(lockdep_rtnl_is_held);
#endif /* #ifdef CONFIG_PROVE_LOCKING */
-static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
+static struct rtnl_link *__rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
static inline int rtm_msgindex(int msgtype)
{
@@ -164,7 +164,8 @@ static int rtnl_register_internal(struct module *owner,
rtnl_doit_func doit, rtnl_dumpit_func dumpit,
unsigned int flags)
{
- struct rtnl_link **tab, *link, *old;
+ struct rtnl_link *link, *old;
+ struct rtnl_link __rcu **tab;
int msgindex;
int ret = -ENOBUFS;
--
2.13.6
^ permalink raw reply related
* RE: [PATCH net-next 4/4] qede: Use NETIF_F_GRO_HW.
From: Yuval Mintz @ 2017-12-04 21:48 UTC (permalink / raw)
To: Michael Chan, davem@davemloft.net
Cc: netdev@vger.kernel.org, Ariel Elior, everest-linux-l2@cavium.com
In-Reply-To: <1512385967-32339-5-git-send-email-michael.chan@broadcom.com>
> Advertise NETIF_F_GRO_HW and turn on or off hardware GRO based on
> NETIF_F_GRO_FW flag.
>
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Cc: everest-linux-l2@cavium.com
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 ++-------
> drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
> 2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c
> b/drivers/net/ethernet/qlogic/qede/qede_filter.c
> index c1a0708..7ee49b4 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
> @@ -901,13 +901,8 @@ int qede_set_features(struct net_device *dev,
> netdev_features_t features)
> netdev_features_t changes = features ^ dev->features;
> bool need_reload = false;
>
> - /* No action needed if hardware GRO is disabled during driver load */
> - if (changes & NETIF_F_GRO) {
> - if (dev->features & NETIF_F_GRO)
> - need_reload = !edev->gro_disable;
> - else
> - need_reload = edev->gro_disable;
> - }
> + if (changes & NETIF_F_GRO_HW)
> + need_reload = true;
This doesn't look right; edev->gro_disable can change due to other
conditions as well - otherwise, it would have been synonymous with
(dev->features & NETIF_F_GRO).
>
> if (need_reload) {
> struct qede_reload_args args;
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c
> b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index 8f9b3eb..b81620e 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -676,7 +676,7 @@ static void qede_init_ndev(struct qede_dev *edev)
> ndev->priv_flags |= IFF_UNICAST_FLT;
>
> /* user-changeble features */
> - hw_features = NETIF_F_GRO | NETIF_F_SG |
> + hw_features = NETIF_F_GRO | NETIF_F_GRO_HW | NETIF_F_SG |
> NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> NETIF_F_TSO | NETIF_F_TSO6;
>
> @@ -1515,7 +1515,7 @@ static void qede_init_fp(struct qede_dev *edev)
> edev->ndev->name, queue_id);
> }
>
> - edev->gro_disable = !(edev->ndev->features & NETIF_F_GRO);
> + edev->gro_disable = !(edev->ndev->features & NETIF_F_GRO_HW);
> }
>
> static int qede_set_real_num_queues(struct qede_dev *edev)
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH net-next 2/4] bnxt_en: Use NETIF_F_GRO_HW.
From: Eric Dumazet @ 2017-12-04 22:00 UTC (permalink / raw)
To: Or Gerlitz, Michael Chan; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CAJ3xEMg5rS+Q=1hV8aMBQJruOH1A7-dScp_8WZwUQTJ7CKcsTg@mail.gmail.com>
On Mon, 2017-12-04 at 23:06 +0200, Or Gerlitz wrote:
> On Mon, Dec 4, 2017 at 8:11 PM, Michael Chan <michael.chan@broadcom.c
> om> wrote:
>
> > All the logic is already in these 3 drivers in the tree. You can
> > see
> > the additional logic in any of these drivers. It's just that these
> > drivers have been using NETIF_F_GRO to turn on this mode in
> > hardware/firmware.
>
> What happens for tcp encaped by some udp tunnel, does
> the HW know how to GRO that?
Yes it does, at least for common encaps (GRE, SIT, IPIP)
^ permalink raw reply
* Re: [PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
From: Eric Dumazet @ 2017-12-04 22:02 UTC (permalink / raw)
To: simo.ghannam, netdev
In-Reply-To: <5a25b7e1.08a5df0a.997e2.12bf@mx.google.com>
On Mon, 2017-12-04 at 22:02 +0100, simo.ghannam@gmail.com wrote:
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> Whenever the sock object is in DCCP_CLOSED state,
> dccp_disconnect() must free
> dccps_hc_tx_ccid and dccps_hc_rx_ccid and set to NULL.
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
From: simo.ghannam @ 2017-12-04 22:08 UTC (permalink / raw)
To: netdev; +Cc: Mohamed Ghannam
In-Reply-To: <1512421326-57291-1-git-send-email-simo.ghannam>
From: Mohamed Ghannam <simo.ghannam@gmail.com>
Whenever the sock object is in DCCP_CLOSED state,
dccp_disconnect() must free dccps_hc_tx_ccid and
dccps_hc_rx_ccid and set to NULL.
Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
---
net/dccp/proto.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index b68168f..9d43c1f 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -259,6 +259,7 @@ int dccp_disconnect(struct sock *sk, int flags)
{
struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_sock *inet = inet_sk(sk);
+ struct dccp_sock *dp = dccp_sk(sk);
int err = 0;
const int old_state = sk->sk_state;
@@ -278,6 +279,10 @@ int dccp_disconnect(struct sock *sk, int flags)
sk->sk_err = ECONNRESET;
dccp_clear_xmit_timers(sk);
+ ccid_hc_rx_delete(dp->dccps_hc_rx_ccid, sk);
+ ccid_hc_tx_delete(dp->dccps_hc_tx_ccid, sk);
+ dp->dccps_hc_rx_ccid = NULL;
+ dp->dccps_hc_tx_ccid = NULL;
__skb_queue_purge(&sk->sk_receive_queue);
__skb_queue_purge(&sk->sk_write_queue);
--
2.7.4
^ permalink raw reply related
* RE: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO_HW
From: Yuval Mintz @ 2017-12-04 22:15 UTC (permalink / raw)
To: Michael Chan, davem@davemloft.net
Cc: netdev@vger.kernel.org, Ariel Elior, everest-linux-l2@cavium.com
In-Reply-To: <1512385967-32339-2-git-send-email-michael.chan@broadcom.com>
> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
> GRO. With this flag, we can now independently turn on or off hardware
> GRO when GRO is on. Hardware GRO guarantees that packets can be
> re-segmented by TSO/GSO to reconstruct the original packet stream.
>
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Cc: everest-linux-l2@cavium.com
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Documentation/networking/netdev-features.txt | 7 +++++++
> include/linux/netdev_features.h | 5 ++++-
> net/core/dev.c | 13 +++++++++++++
> net/core/ethtool.c | 1 +
> 4 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/networking/netdev-features.txt
> b/Documentation/networking/netdev-features.txt
> index 7413eb0..d76d332 100644
> --- a/Documentation/networking/netdev-features.txt
> +++ b/Documentation/networking/netdev-features.txt
> @@ -163,3 +163,10 @@ This requests that the NIC receive all possible
> frames, including errored
> frames (such as bad FCS, etc). This can be helpful when sniffing a link with
> bad packets on it. Some NICs may receive more packets if also put into
> normal
> PROMISC mode.
> +
> +* rx-gro-hw
> +
> +This requests that the NIC enables Hardware GRO (generic receive offload).
> +Hardware GRO is basically the exact reverse of TSO, and is generally
> +stricter than Hardware LRO. A packet stream merged by Hardware GRO
> must
> +be re-segmentable by GSO or TSO back to the exact packet stream.
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index dc8b489..d18ef6f 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -77,6 +77,8 @@ enum {
> NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum
> offload */
> NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP
> tunnels */
>
> + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive
> offload */
> +
> /*
> * Add your fresh new feature above and remember to update
> * netdev_features_strings[] in net/core/ethtool.c and maybe
> @@ -96,6 +98,7 @@ enum {
> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
> #define NETIF_F_FSO __NETIF_F(FSO)
> #define NETIF_F_GRO __NETIF_F(GRO)
> +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
> #define NETIF_F_GSO __NETIF_F(GSO)
> #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
> #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
> @@ -193,7 +196,7 @@ enum {
> * If upper/master device has these features disabled, they must be disabled
> * on all lower/slave devices as well.
> */
> -#define NETIF_F_UPPER_DISABLES NETIF_F_LRO
> +#define NETIF_F_UPPER_DISABLES (NETIF_F_LRO | NETIF_F_GRO_HW)
>
> /* changeable features with no special hardware requirements */
> #define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 30b5fe3..09c2ad0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7392,6 +7392,19 @@ static netdev_features_t
> netdev_fix_features(struct net_device *dev,
> features &= ~dev->gso_partial_features;
> }
>
> + if (features & NETIF_F_GRO_HW) {
> + /* Hardware GRO depends on GRO. */
> + if (!(features & NETIF_F_GRO)) {
While at it, perhaps also make it dependent on NETIF_F_RXCSUM?
> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since
> no GRO feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
> + /* Hardware GRO and LRO are mutually exclusive. */
> + if (features & NETIF_F_LRO) {
> + netdev_dbg(dev, "Dropping NETIF_F_LRO since
> GRO_HW is set.\n");
> + features &= ~NETIF_F_LRO;
Isn't this considered to be breaking an existing API?
After this, while NETIF_F_GRO_HW is published an application trying to
set NETIF_F_LRO and then query its state would discover it failed
[while previously it could have succeeded, such as for bnx2]
While I understand the need to make sure core doesn't enable
two competing aggregation offloads, why make GRO_HW > LRO?
I understand it's probably the better one, but until LRO gets deprecated
isn't it safer to do this limitation the opposite way?
I.e., make sure NETIF_F_GRO_HW can't be set as long as NETIF_F_LRO is set?
> + }
> + }
> +
> return features;
> }
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index f8fcf45..50a7920 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev,
> struct ethtool_ts_info *info)
> [NETIF_F_LLTX_BIT] = "tx-lockless",
> [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> [NETIF_F_GRO_BIT] = "rx-gro",
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> [NETIF_F_LRO_BIT] = "rx-lro",
>
> [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> --
> 1.8.3.1
^ permalink raw reply
* [PATCH net-next] bpf: move bpf csum flag check
From: William Tu @ 2017-12-04 22:18 UTC (permalink / raw)
To: netdev
trivial move the BPF_F_ZERO_CSUM_TX check right below the
'flags & BPF_F_DONT_FRAGMENT', so common tun_flags handling
is logically together.
Signed-off-by: William Tu <u9012063@gmail.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
net/core/filter.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 8ec5a504eb28..4d644ad17457 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3013,6 +3013,8 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
info->key.tun_flags = TUNNEL_KEY | TUNNEL_CSUM | TUNNEL_NOCACHE;
if (flags & BPF_F_DONT_FRAGMENT)
info->key.tun_flags |= TUNNEL_DONT_FRAGMENT;
+ if (flags & BPF_F_ZERO_CSUM_TX)
+ info->key.tun_flags &= ~TUNNEL_CSUM;
info->key.tun_id = cpu_to_be64(from->tunnel_id);
info->key.tos = from->tunnel_tos;
@@ -3028,9 +3030,6 @@ BPF_CALL_4(bpf_skb_set_tunnel_key, struct sk_buff *, skb,
info->key.u.ipv4.dst = cpu_to_be32(from->remote_ipv4);
}
- if (flags & BPF_F_ZERO_CSUM_TX)
- info->key.tun_flags &= ~TUNNEL_CSUM;
-
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 1/2 v6] net: ethernet: Add DT bindings for the Gemini ethernet
From: Rob Herring @ 2017-12-04 22:30 UTC (permalink / raw)
To: Linus Walleij
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S . Miller,
Michał Mirosław, Janos Laube, Paulius Zaleckas,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hans Ulli Kroll, Florian Fainelli,
devicetree-u79uwXL29TY76Z2rM5mHXA, Tobias Waldvogel
In-Reply-To: <20171202110640.5284-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Sat, Dec 02, 2017 at 12:06:39PM +0100, Linus Walleij wrote:
> This adds the device tree bindings for the Gemini ethernet
> controller. It is pretty straight-forward, using standard
> bindings and modelling the two child ports as child devices
> under the parent ethernet controller device.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Tobias Waldvogel <tobias.waldvogel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Michał Mirosław <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> .../bindings/net/cortina,gemini-ethernet.txt | 92 ++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
>
> diff --git a/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
> new file mode 100644
> index 000000000000..35fa3abd1c73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cortina,gemini-ethernet.txt
> @@ -0,0 +1,92 @@
> +Cortina Systems Gemini Ethernet Controller
> +==========================================
> +
> +This ethernet controller is found in the Gemini SoC family:
> +StorLink SL3512 and SL3516, also known as Cortina Systems
> +CS3512 and CS3516.
> +
> +Required properties:
> +- compatible: must be "cortina,gemini-ethernet"
> +- reg: must contain the global registers and the V-bit and A-bit
> + memory areas, in total three register sets.
> +- syscon: a phandle to the system controller
> +- #address-cells: must be specified, must be <1>
> +- #size-cells: must be specified, must be <1>
> +- ranges: should be state like this giving a 1:1 address translation
> + for the subnodes
> +
> +The subnodes represents the two ethernet ports in this device.
> +They are not independent of each other since they share resources
> +in the parent node, and are thus children.
> +
> +Required subnodes:
> +- port0: contains the resources for ethernet port 0
> +- port1: contains the resources for ethernet port 1
> +
> +Required subnode properties:
> +- compatible: must be "cortina,gemini-ethernet-port"
> +- reg: must contain two register areas: the DMA/TOE memory and
> + the GMAC memory area of the port
> +- interrupts: should contain the interrupt line of the port.
> + this is nominally a level interrupt active high.
> +- resets: this must provide an SoC-integrated reset line for
> + the port.
> +- clocks: this should contain a handle to the PCLK clock for
> + clocking the silicon in this port
> +- clock-names: must be "PCLK"
> +
> +Optional subnode properties:
> +- phy-mode: see ethernet.txt
> +- phy-handle: see ethernet.txt
> +
> +Example:
> +
> +mdio-bus {
> + (...)
> + phy0: ethernet-phy@1 {
> + reg = <1>;
> + device_type = "ethernet-phy";
> + };
> + phy1: ethernet-phy@3 {
> + reg = <3>;
> + device_type = "ethernet-phy";
> + };
> +};
> +
> +
> +ethernet@60000000 {
> + compatible = "cortina,gemini-ethernet";
> + reg = <0x60000000 0x4000>, /* Global registers, queue */
> + <0x60004000 0x2000>, /* V-bit */
> + <0x60006000 0x2000>; /* A-bit */
> + syscon = <&syscon>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
Would be better to define the actual range used by child nodes.
> +
> + gmac0: port0 {
Needs a unit-address. Building with W=1 (or W=2) will tell you this.
As port is used by the OF graph binding, use ethernet-port@... instead.
> + compatible = "cortina,gemini-ethernet-port";
> + reg = <0x60008000 0x2000>, /* Port 0 DMA/TOE */
> + <0x6000a000 0x2000>; /* Port 0 GMAC */
> + interrupt-parent = <&intcon>;
> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&syscon GEMINI_RESET_GMAC0>;
> + clocks = <&syscon GEMINI_CLK_GATE_GMAC0>;
> + clock-names = "PCLK";
> + phy-mode = "rgmii";
> + phy-handle = <&phy0>;
> + };
> +
> + gmac1: port1 {
> + compatible = "cortina,gemini-ethernet-port";
> + reg = <0x6000c000 0x2000>, /* Port 1 DMA/TOE */
> + <0x6000e000 0x2000>; /* Port 1 GMAC */
> + interrupt-parent = <&intcon>;
> + interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> + resets = <&syscon GEMINI_RESET_GMAC1>;
> + clocks = <&syscon GEMINI_CLK_GATE_GMAC1>;
> + clock-names = "PCLK";
> + phy-mode = "rgmii";
> + phy-handle = <&phy1>;
> + };
> +};
> --
> 2.14.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: Introduce NETIF_F_GRO_HW
From: Michael Chan @ 2017-12-04 22:31 UTC (permalink / raw)
To: Yuval Mintz
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ariel Elior,
everest-linux-l2@cavium.com
In-Reply-To: <VI1PR05MB35202FBCCC9C4B06F5C9148FBF3C0@VI1PR05MB3520.eurprd05.prod.outlook.com>
On Mon, Dec 4, 2017 at 2:15 PM, Yuval Mintz <yuvalm@mellanox.com> wrote:
>> @@ -96,6 +98,7 @@ enum {
>> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
>> #define NETIF_F_FSO __NETIF_F(FSO)
>> #define NETIF_F_GRO __NETIF_F(GRO)
>> +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
>> #define NETIF_F_GSO __NETIF_F(GSO)
>> #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
>> #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
>> @@ -193,7 +196,7 @@ enum {
>> * If upper/master device has these features disabled, they must be disabled
>> * on all lower/slave devices as well.
>> */
>> -#define NETIF_F_UPPER_DISABLES NETIF_F_LRO
>> +#define NETIF_F_UPPER_DISABLES (NETIF_F_LRO | NETIF_F_GRO_HW)
>>
>> /* changeable features with no special hardware requirements */
>> #define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 30b5fe3..09c2ad0 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -7392,6 +7392,19 @@ static netdev_features_t
>> netdev_fix_features(struct net_device *dev,
>> features &= ~dev->gso_partial_features;
>> }
>>
>> + if (features & NETIF_F_GRO_HW) {
>> + /* Hardware GRO depends on GRO. */
>> + if (!(features & NETIF_F_GRO)) {
>
> While at it, perhaps also make it dependent on NETIF_F_RXCSUM?
OK. Makes sense.
>
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since
>> no GRO feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>> + /* Hardware GRO and LRO are mutually exclusive. */
>> + if (features & NETIF_F_LRO) {
>> + netdev_dbg(dev, "Dropping NETIF_F_LRO since
>> GRO_HW is set.\n");
>> + features &= ~NETIF_F_LRO;
>
> Isn't this considered to be breaking an existing API?
> After this, while NETIF_F_GRO_HW is published an application trying to
> set NETIF_F_LRO and then query its state would discover it failed
> [while previously it could have succeeded, such as for bnx2]
>
> While I understand the need to make sure core doesn't enable
> two competing aggregation offloads, why make GRO_HW > LRO?
> I understand it's probably the better one, but until LRO gets deprecated
> isn't it safer to do this limitation the opposite way?
> I.e., make sure NETIF_F_GRO_HW can't be set as long as NETIF_F_LRO is set?
I am just following precedents in the netdev_fix_features() logic to
drop incompatible features. I can make LRO and GRO_HW have equal
standing by dropping the other when one is set. So if I do that, user
will be able to always enable LRO. The code will just drop GRO_HW if
it is set.
^ permalink raw reply
* Re: [PATCH net-next] bpf: Add access to snd_cwnd and others in sock_ops
From: Daniel Borkmann @ 2017-12-04 22:32 UTC (permalink / raw)
To: Lawrence Brakmo, netdev
Cc: Alexei Starovoitov, Kernel Team, Blake Matheny, Vlad Dumitrescu
In-Reply-To: <20171201181504.1040223-1-brakmo@fb.com>
Hi Lawrence,
On 12/01/2017 07:15 PM, Lawrence Brakmo wrote:
> Adds read access to snd_cwnd and srtt_us fields of tcp_sock. Since these
> fields are only valid if the socket associated with the sock_ops program
> call is a full socket, the field is_fullsock is also added to the
> bpf_sock_ops struct. If the socket is not a full socket, reading these
> fields returns 0.
>
> Note that in most cases it will not be necessary to check is_fullsock to
> know if there is a full socket. The context of the call, as specified by
> the 'op' field, can sometimes determine whether there is a full socket.
>
> The struct bpf_sock_ops has the following fields added:
>
> __u32 is_fullsock; /* Some TCP fields are only valid if
> * there is a full socket. If not, the
> * fields read as zero.
> */
> __u32 snd_cwnd;
> __u32 srtt_us; /* Averaged RTT << 3 in usecs */
>
> There is a new macro, SOCK_OPS_GET_TCP32(NAME), to make it easier to add
> read access to more 32 bit tcp_sock fields.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> include/linux/filter.h | 1 +
> include/net/tcp.h | 6 ++++--
> include/uapi/linux/bpf.h | 6 ++++++
> net/core/filter.c | 36 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 80b5b48..0062302 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -985,6 +985,7 @@ struct bpf_sock_ops_kern {
> u32 reply;
> u32 replylong[4];
> };
> + u32 is_fullsock;
> };
>
> #endif /* __LINUX_FILTER_H__ */
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4e09398..89a6560 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2012,10 +2012,12 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
> struct bpf_sock_ops_kern sock_ops;
> int ret;
>
> - if (sk_fullsock(sk))
> + memset(&sock_ops, 0, sizeof(sock_ops));
> + if (sk_fullsock(sk)) {
> + sock_ops.is_fullsock = 1;
> sock_owned_by_me(sk);
> + }
>
> - memset(&sock_ops, 0, sizeof(sock_ops));
> sock_ops.sk = sk;
> sock_ops.op = op;
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4c223ab..80d62e8 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -941,6 +941,12 @@ struct bpf_sock_ops {
> __u32 local_ip6[4]; /* Stored in network byte order */
> __u32 remote_port; /* Stored in network byte order */
> __u32 local_port; /* stored in host byte order */
> + __u32 is_fullsock; /* Some TCP fields are only valid if
> + * there is a full socket. If not, the
> + * fields read as zero.
> + */
Just a question for clarification: do you need to have this in
the uapi struct as well? Meaning, do you have a specific use case
where you do this check out of the BPF program given the below
two snd_cwnd/srtt_us check this internally in the ctx rewrite?
Do you plan to reuse the ctx assignment also for bpf_setsockopt()
and bpf_getsockopt() internally?
> + __u32 snd_cwnd;
> + __u32 srtt_us; /* Averaged RTT << 3 in usecs */
> };
>
> /* List of known BPF sock_ops operators.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6a85e67..bf31236 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4437,6 +4437,42 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
> *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->dst_reg,
> offsetof(struct sock_common, skc_num));
> break;
> +
> + case offsetof(struct bpf_sock_ops, is_fullsock):
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
> + struct bpf_sock_ops_kern,
> + is_fullsock),
> + si->dst_reg, si->src_reg,
> + offsetof(struct bpf_sock_ops_kern,
> + is_fullsock));
> + break;
> +
> +/* Helper macro for adding read access to tcp_sock fields. */
> +#define SOCK_OPS_GET_TCP32(FIELD_NAME) \
> + do { \
> + BUILD_BUG_ON(FIELD_SIZEOF(struct tcp_sock, FIELD_NAME) != 4); \
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF( \
> + struct bpf_sock_ops_kern, \
> + is_fullsock), \
> + si->dst_reg, si->src_reg, \
> + offsetof(struct bpf_sock_ops_kern, \
> + is_fullsock)); \
> + *insn++ = BPF_JMP_IMM(BPF_JEQ, si->dst_reg, 0, 2); \
> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF( \
> + struct bpf_sock_ops_kern, sk),\
> + si->dst_reg, si->src_reg, \
> + offsetof(struct bpf_sock_ops_kern, sk));\
> + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg, \
> + offsetof(struct tcp_sock, FIELD_NAME)); \
> + } while (0)
> +
> + case offsetof(struct bpf_sock_ops, snd_cwnd):
> + SOCK_OPS_GET_TCP32(snd_cwnd);
> + break;
> +
> + case offsetof(struct bpf_sock_ops, srtt_us):
> + SOCK_OPS_GET_TCP32(srtt_us);
> + break;
> }
> return insn - insn_buf;
> }
>
^ permalink raw reply
* [PATCH net-next 00/10] nfp: enhanced debug dump via ethtool
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
Add debug dump implementation to the NFP driver. This makes use of
existing ethtool infrastructure. ethtool -W is used to select the dump
level and ethtool -w is used to dump NFP state.
The existing behaviour of dump level 0, dumping the arm.diag resource, is
preserved. Dump levels greater than 0 are implemented by this patchset and
optionally supported by firmware providing a _abi_dump_spec rtsym. This
rtsym provides a specification, in TLV format, of the information to be
dumped from the NFP at each supported dump level.
Dumps are also structured using a TLVs. They consist a prolog and the data
described int he corresponding dump.
Carl Heymann (10):
nfp: debug dump ethtool ops
nfp: load debug dump spec
nfp: dump prolog
nfp: dumpspec TLV traversal
nfp: dump rtsyms
nfp: dump all hwinfo
nfp: dump single hwinfo field by key
nfp: dump firmware name
nfp: dump CPP, XPB and direct ME CSRs
nfp: dump indirect ME CSRs
drivers/net/ethernet/netronome/nfp/Makefile | 1 +
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 10 +
drivers/net/ethernet/netronome/nfp/nfp_main.c | 6 +
drivers/net/ethernet/netronome/nfp/nfp_main.h | 29 +
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 787 +++++++++++++++++++++
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 55 +-
drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h | 2 +
.../ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c | 10 +
8 files changed, 892 insertions(+), 8 deletions(-)
create mode 100644 drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
--
2.11.0
^ permalink raw reply
* [PATCH net-next 01/10] nfp: debug dump ethtool ops
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Skeleton code to perform a binary debug dump via ethtoolops
"set_dump", "get_dump_flags" and "get_dump_data", i.e. the ethtool
-W/w mechanism.
- Skeleton functions for debugdump operations provided.
- An integer "dump level" can be specified, this is stored between
ethtool invocations. Dump level 0 is still the "arm.diag" resource for
backward compatibility. Other dump levels each define a set of state
information to include in the dump, driven by a spec from FW.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/Makefile | 1 +
drivers/net/ethernet/netronome/nfp/nfp_main.c | 6 +++
drivers/net/ethernet/netronome/nfp/nfp_main.h | 29 ++++++++++++
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 54 +++++++++++++++++++++
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 55 ++++++++++++++++++----
5 files changed, 137 insertions(+), 8 deletions(-)
create mode 100644 drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
diff --git a/drivers/net/ethernet/netronome/nfp/Makefile b/drivers/net/ethernet/netronome/nfp/Makefile
index 24c4408b5734..6e5ef984398b 100644
--- a/drivers/net/ethernet/netronome/nfp/Makefile
+++ b/drivers/net/ethernet/netronome/nfp/Makefile
@@ -22,6 +22,7 @@ nfp-objs := \
nfp_hwmon.o \
nfp_main.o \
nfp_net_common.o \
+ nfp_net_debugdump.o \
nfp_net_ethtool.o \
nfp_net_main.o \
nfp_net_repr.o \
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 35eaccbece36..0953fa8f3109 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -45,6 +45,7 @@
#include <linux/pci.h>
#include <linux/firmware.h>
#include <linux/vermagic.h>
+#include <linux/vmalloc.h>
#include <net/devlink.h>
#include "nfpcore/nfp.h"
@@ -509,6 +510,9 @@ static int nfp_pci_probe(struct pci_dev *pdev,
pf->mip = nfp_mip_open(pf->cpp);
pf->rtbl = __nfp_rtsym_table_read(pf->cpp, pf->mip);
+ pf->dump_flag = NFP_DUMP_NSP_DIAG;
+ pf->dumpspec = nfp_net_dump_load_dumpspec(pf->cpp, pf->rtbl);
+
err = nfp_pcie_sriov_read_nfd_limit(pf);
if (err)
goto err_fw_unload;
@@ -544,6 +548,7 @@ static int nfp_pci_probe(struct pci_dev *pdev,
nfp_fw_unload(pf);
kfree(pf->eth_tbl);
kfree(pf->nspi);
+ vfree(pf->dumpspec);
err_devlink_unreg:
devlink_unregister(devlink);
err_hwinfo_free:
@@ -579,6 +584,7 @@ static void nfp_pci_remove(struct pci_dev *pdev)
devlink_unregister(devlink);
+ vfree(pf->dumpspec);
kfree(pf->rtbl);
nfp_mip_close(pf->mip);
if (pf->fw_loaded)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index be0ee59f2eb9..add46e28212b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -39,6 +39,7 @@
#ifndef NFP_MAIN_H
#define NFP_MAIN_H
+#include <linux/ethtool.h>
#include <linux/list.h>
#include <linux/types.h>
#include <linux/msi.h>
@@ -62,6 +63,17 @@ struct nfp_port;
struct nfp_rtsym_table;
/**
+ * struct nfp_dumpspec - NFP FW dump specification structure
+ * @size: Size of the data
+ * @data: Sequence of TLVs, each being an instruction to dump some data
+ * from FW
+ */
+struct nfp_dumpspec {
+ u32 size;
+ u8 data[0];
+};
+
+/**
* struct nfp_pf - NFP PF-specific device structure
* @pdev: Backpointer to PCI device
* @cpp: Pointer to the CPP handle
@@ -83,6 +95,9 @@ struct nfp_rtsym_table;
* @mip: MIP handle
* @rtbl: RTsym table
* @hwinfo: HWInfo table
+ * @dumpspec: Debug dump specification
+ * @dump_flag: Store dump flag between set_dump and get_dump_flag
+ * @dump_len: Store dump length between set_dump and get_dump_flag
* @eth_tbl: NSP ETH table
* @nspi: NSP identification info
* @hwmon_dev: pointer to hwmon device
@@ -124,6 +139,9 @@ struct nfp_pf {
const struct nfp_mip *mip;
struct nfp_rtsym_table *rtbl;
struct nfp_hwinfo *hwinfo;
+ struct nfp_dumpspec *dumpspec;
+ u32 dump_flag;
+ u32 dump_len;
struct nfp_eth_table *eth_tbl;
struct nfp_nsp_identify *nspi;
@@ -157,4 +175,15 @@ void nfp_net_get_mac_addr(struct nfp_pf *pf, struct nfp_port *port);
bool nfp_ctrl_tx(struct nfp_net *nn, struct sk_buff *skb);
+enum nfp_dump_diag {
+ NFP_DUMP_NSP_DIAG = 0,
+};
+
+struct nfp_dumpspec *
+nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl);
+s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
+ u32 flag);
+int nfp_net_dump_populate_buffer(struct nfp_pf *pf, struct nfp_dumpspec *spec,
+ struct ethtool_dump *dump_param, void *dest);
+
#endif /* NFP_MAIN_H */
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
new file mode 100644
index 000000000000..a7061b6c609d
--- /dev/null
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2017 Netronome Systems, Inc.
+ *
+ * This software is dual licensed under the GNU General License Version 2,
+ * June 1991 as shown in the file COPYING in the top-level directory of this
+ * source tree or the BSD 2-Clause License provided below. You have the
+ * option to license this software under the complete terms of either license.
+ *
+ * The BSD 2-Clause License:
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/ethtool.h>
+
+#include "nfp_main.h"
+
+struct nfp_dumpspec *
+nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
+{
+ return NULL;
+}
+
+s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
+ u32 flag)
+{
+ return -EOPNOTSUPP;
+}
+
+int nfp_net_dump_populate_buffer(struct nfp_pf *pf, struct nfp_dumpspec *spec,
+ struct ethtool_dump *dump_param, void *dest)
+{
+ return -EOPNOTSUPP;
+}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 2801ecd09eab..2cde0eb00ee3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -51,14 +51,11 @@
#include "nfpcore/nfp.h"
#include "nfpcore/nfp_nsp.h"
#include "nfp_app.h"
+#include "nfp_main.h"
#include "nfp_net_ctrl.h"
#include "nfp_net.h"
#include "nfp_port.h"
-enum nfp_dump_diag {
- NFP_DUMP_NSP_DIAG = 0,
-};
-
struct nfp_et_stat {
char name[ETH_GSTRING_LEN];
int off;
@@ -1066,15 +1063,34 @@ nfp_dump_nsp_diag(struct nfp_app *app, struct ethtool_dump *dump, void *buffer)
return ret;
}
+/* Set the dump flag/level. Calculate the dump length for flag > 0 only (new TLV
+ * based dumps), since flag 0 (default) calculates the length in
+ * nfp_app_get_dump_flag(), and we need to support triggering a level 0 dump
+ * without setting the flag first, for backward compatibility.
+ */
static int nfp_app_set_dump(struct net_device *netdev, struct ethtool_dump *val)
{
struct nfp_app *app = nfp_app_from_netdev(netdev);
+ s64 len;
if (!app)
return -EOPNOTSUPP;
- if (val->flag != NFP_DUMP_NSP_DIAG)
- return -EINVAL;
+ if (val->flag == NFP_DUMP_NSP_DIAG) {
+ app->pf->dump_flag = val->flag;
+ return 0;
+ }
+
+ if (!app->pf->dumpspec)
+ return -EOPNOTSUPP;
+
+ len = nfp_net_dump_calculate_size(app->pf, app->pf->dumpspec,
+ val->flag);
+ if (len < 0)
+ return len;
+
+ app->pf->dump_flag = val->flag;
+ app->pf->dump_len = len;
return 0;
}
@@ -1082,14 +1098,37 @@ static int nfp_app_set_dump(struct net_device *netdev, struct ethtool_dump *val)
static int
nfp_app_get_dump_flag(struct net_device *netdev, struct ethtool_dump *dump)
{
- return nfp_dump_nsp_diag(nfp_app_from_netdev(netdev), dump, NULL);
+ struct nfp_app *app = nfp_app_from_netdev(netdev);
+
+ if (!app)
+ return -EOPNOTSUPP;
+
+ if (app->pf->dump_flag == NFP_DUMP_NSP_DIAG)
+ return nfp_dump_nsp_diag(app, dump, NULL);
+
+ dump->flag = app->pf->dump_flag;
+ dump->len = app->pf->dump_len;
+
+ return 0;
}
static int
nfp_app_get_dump_data(struct net_device *netdev, struct ethtool_dump *dump,
void *buffer)
{
- return nfp_dump_nsp_diag(nfp_app_from_netdev(netdev), dump, buffer);
+ struct nfp_app *app = nfp_app_from_netdev(netdev);
+
+ if (!app)
+ return -EOPNOTSUPP;
+
+ if (app->pf->dump_flag == NFP_DUMP_NSP_DIAG)
+ return nfp_dump_nsp_diag(app, dump, buffer);
+
+ dump->flag = app->pf->dump_flag;
+ dump->len = app->pf->dump_len;
+
+ return nfp_net_dump_populate_buffer(app->pf, app->pf->dumpspec, dump,
+ buffer);
}
static int nfp_net_set_coalesce(struct net_device *netdev,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 02/10] nfp: load debug dump spec
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
Load the TLV-based binary specification of what needs to be included in
a dump, from the "_abi_dump_spec" rtsymbol. If the symbol is not defined,
then dumps for levels >= 1 are not supported.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 34 +++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index a7061b6c609d..15b6623ef0b2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -32,13 +32,45 @@
*/
#include <linux/ethtool.h>
+#include <linux/vmalloc.h>
#include "nfp_main.h"
+#include "nfpcore/nfp.h"
+#include "nfpcore/nfp_nffw.h"
+
+#define NFP_DUMP_SPEC_RTSYM "_abi_dump_spec"
struct nfp_dumpspec *
nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
{
- return NULL;
+ const struct nfp_rtsym *specsym;
+ struct nfp_dumpspec *dumpspec;
+ int bytes_read;
+ u32 cpp_id;
+
+ specsym = nfp_rtsym_lookup(rtbl, NFP_DUMP_SPEC_RTSYM);
+ if (!specsym)
+ return NULL;
+
+ /* expected size of this buffer is in the order of tens of kilobytes */
+ dumpspec = vmalloc(sizeof(*dumpspec) + specsym->size);
+ if (!dumpspec)
+ return NULL;
+
+ dumpspec->size = specsym->size;
+
+ cpp_id = NFP_CPP_ISLAND_ID(specsym->target, NFP_CPP_ACTION_RW, 0,
+ specsym->domain);
+
+ bytes_read = nfp_cpp_read(cpp, cpp_id, specsym->addr, dumpspec->data,
+ specsym->size);
+ if (bytes_read != specsym->size) {
+ vfree(dumpspec);
+ nfp_warn(cpp, "Debug dump specification read failed.\n");
+ return NULL;
+ }
+
+ return dumpspec;
}
s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 03/10] nfp: dump prolog
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Use a TLV structure, with the typed chunks aligned to 8-byte sizes.
- Dump numeric fields as big-endian.
- Prolog contains the dump level.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 88 +++++++++++++++++++++-
1 file changed, 86 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 15b6623ef0b2..f05566fd12a2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -40,6 +40,37 @@
#define NFP_DUMP_SPEC_RTSYM "_abi_dump_spec"
+#define ALIGN8(x) ALIGN(x, 8)
+
+enum nfp_dumpspec_type {
+ NFP_DUMPSPEC_TYPE_PROLOG = 10000,
+};
+
+/* The following structs must be carefully aligned so that they can be used to
+ * interpret the binary dumpspec and populate the dump data in a deterministic
+ * way.
+ */
+
+/* generic type plus length */
+struct nfp_dump_tl {
+ __be32 type;
+ __be32 length; /* chunk length to follow, aligned to 8 bytes */
+ char data[0];
+};
+
+struct nfp_dump_prolog {
+ struct nfp_dump_tl tl;
+ __be32 dump_level;
+};
+
+/* to track state during debug dump creation TLV traversal */
+struct nfp_dump_state {
+ u32 requested_level; /* input param */
+ u32 dumped_size; /* adds up to size of dumped data */
+ u32 buf_size; /* size of buffer pointer to by p */
+ void *p; /* current point in dump buffer */
+};
+
struct nfp_dumpspec *
nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
{
@@ -76,11 +107,64 @@ nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
u32 flag)
{
- return -EOPNOTSUPP;
+ return ALIGN8(sizeof(struct nfp_dump_prolog));
+}
+
+static int nfp_add_tlv(u32 type, u32 total_tlv_sz, struct nfp_dump_state *dump)
+{
+ struct nfp_dump_tl *tl = dump->p;
+
+ if (total_tlv_sz > dump->buf_size)
+ return -ENOSPC;
+
+ if (dump->buf_size - total_tlv_sz < dump->dumped_size)
+ return -ENOSPC;
+
+ tl->type = cpu_to_be32(type);
+ tl->length = cpu_to_be32(total_tlv_sz - sizeof(*tl));
+
+ dump->dumped_size += total_tlv_sz;
+ dump->p += total_tlv_sz;
+
+ return 0;
+}
+
+static int nfp_dump_populate_prolog(struct nfp_dump_state *dump)
+{
+ struct nfp_dump_prolog *prolog = dump->p;
+ u32 total_size;
+ int err;
+
+ total_size = ALIGN8(sizeof(*prolog));
+
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_PROLOG, total_size, dump);
+ if (err)
+ return err;
+
+ prolog->dump_level = cpu_to_be32(dump->requested_level);
+
+ return 0;
}
int nfp_net_dump_populate_buffer(struct nfp_pf *pf, struct nfp_dumpspec *spec,
struct ethtool_dump *dump_param, void *dest)
{
- return -EOPNOTSUPP;
+ struct nfp_dump_state dump;
+ int err;
+
+ dump.requested_level = dump_param->flag;
+ dump.dumped_size = 0;
+ dump.p = dest;
+ dump.buf_size = dump_param->len;
+
+ err = nfp_dump_populate_prolog(&dump);
+ if (err)
+ return err;
+
+ /* Set size of actual dump, to trigger warning if different from
+ * calculated size.
+ */
+ dump_param->len = dump.dumped_size;
+
+ return 0;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 04/10] nfp: dumpspec TLV traversal
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Perform dumpspec traversals for calculating size and populating the
dump.
- Initially, wrap all spec TLVs in dump error TLVs (changed by later
patches in the series).
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 154 ++++++++++++++++++++-
1 file changed, 153 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index f05566fd12a2..d52e01ca6621 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -44,6 +44,7 @@
enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
+ NFP_DUMPSPEC_TYPE_ERROR = 10001,
};
/* The following structs must be carefully aligned so that they can be used to
@@ -63,6 +64,19 @@ struct nfp_dump_prolog {
__be32 dump_level;
};
+struct nfp_dump_error {
+ struct nfp_dump_tl tl;
+ __be32 error;
+ char padding[4];
+ char spec[0];
+};
+
+/* to track state through debug size calculation TLV traversal */
+struct nfp_level_size {
+ u32 requested_level; /* input */
+ u32 total_size; /* output */
+};
+
/* to track state during debug dump creation TLV traversal */
struct nfp_dump_state {
u32 requested_level; /* input param */
@@ -71,6 +85,43 @@ struct nfp_dump_state {
void *p; /* current point in dump buffer */
};
+typedef int (*nfp_tlv_visit)(struct nfp_pf *pf, struct nfp_dump_tl *tl,
+ void *param);
+
+static int
+nfp_traverse_tlvs(struct nfp_pf *pf, void *data, u32 data_length, void *param,
+ nfp_tlv_visit tlv_visit)
+{
+ long long remaining = data_length;
+ struct nfp_dump_tl *tl;
+ u32 total_tlv_size;
+ void *p = data;
+ int err;
+
+ while (remaining >= sizeof(*tl)) {
+ tl = p;
+ if (!tl->type && !tl->length)
+ break;
+
+ if (be32_to_cpu(tl->length) > remaining - sizeof(*tl))
+ return -EINVAL;
+
+ total_tlv_size = sizeof(*tl) + be32_to_cpu(tl->length);
+
+ /* Spec TLVs should be aligned to 4 bytes. */
+ if (total_tlv_size % 4 != 0)
+ return -EINVAL;
+
+ p += total_tlv_size;
+ remaining -= total_tlv_size;
+ err = tlv_visit(pf, tl, param);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
struct nfp_dumpspec *
nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
{
@@ -104,10 +155,55 @@ nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
return dumpspec;
}
+static int nfp_dump_error_tlv_size(struct nfp_dump_tl *spec)
+{
+ return ALIGN8(sizeof(struct nfp_dump_error) + sizeof(*spec) +
+ be32_to_cpu(spec->length));
+}
+
+static int
+nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
+{
+ u32 *size = param;
+
+ switch (be32_to_cpu(tl->type)) {
+ default:
+ *size += nfp_dump_error_tlv_size(tl);
+ break;
+ }
+
+ return 0;
+}
+
+static int
+nfp_calc_specific_level_size(struct nfp_pf *pf, struct nfp_dump_tl *dump_level,
+ void *param)
+{
+ struct nfp_level_size *lev_sz = param;
+
+ if (be32_to_cpu(dump_level->type) != lev_sz->requested_level)
+ return 0;
+
+ return nfp_traverse_tlvs(pf, dump_level->data,
+ be32_to_cpu(dump_level->length),
+ &lev_sz->total_size, nfp_add_tlv_size);
+}
+
s64 nfp_net_dump_calculate_size(struct nfp_pf *pf, struct nfp_dumpspec *spec,
u32 flag)
{
- return ALIGN8(sizeof(struct nfp_dump_prolog));
+ struct nfp_level_size lev_sz;
+ int err;
+
+ lev_sz.requested_level = flag;
+ lev_sz.total_size = ALIGN8(sizeof(struct nfp_dump_prolog));
+
+ err = nfp_traverse_tlvs(pf, spec->data, spec->size, &lev_sz,
+ nfp_calc_specific_level_size);
+ if (err)
+ return err;
+
+ return lev_sz.total_size;
}
static int nfp_add_tlv(u32 type, u32 total_tlv_sz, struct nfp_dump_state *dump)
@@ -129,6 +225,57 @@ static int nfp_add_tlv(u32 type, u32 total_tlv_sz, struct nfp_dump_state *dump)
return 0;
}
+static int
+nfp_dump_error_tlv(struct nfp_dump_tl *spec, int error,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_error *dump_header = dump->p;
+ u32 total_spec_size, total_size;
+ int err;
+
+ total_spec_size = sizeof(*spec) + be32_to_cpu(spec->length);
+ total_size = ALIGN8(sizeof(*dump_header) + total_spec_size);
+
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_ERROR, total_size, dump);
+ if (err)
+ return err;
+
+ dump_header->error = cpu_to_be32(error);
+ memcpy(dump_header->spec, spec, total_spec_size);
+
+ return 0;
+}
+
+static int
+nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
+{
+ struct nfp_dump_state *dump = param;
+ int err;
+
+ switch (be32_to_cpu(tl->type)) {
+ default:
+ err = nfp_dump_error_tlv(tl, -EOPNOTSUPP, dump);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int
+nfp_dump_specific_level(struct nfp_pf *pf, struct nfp_dump_tl *dump_level,
+ void *param)
+{
+ struct nfp_dump_state *dump = param;
+
+ if (be32_to_cpu(dump_level->type) != dump->requested_level)
+ return 0;
+
+ return nfp_traverse_tlvs(pf, dump_level->data,
+ be32_to_cpu(dump_level->length), dump,
+ nfp_dump_for_tlv);
+}
+
static int nfp_dump_populate_prolog(struct nfp_dump_state *dump)
{
struct nfp_dump_prolog *prolog = dump->p;
@@ -161,6 +308,11 @@ int nfp_net_dump_populate_buffer(struct nfp_pf *pf, struct nfp_dumpspec *spec,
if (err)
return err;
+ err = nfp_traverse_tlvs(pf, spec->data, spec->size, &dump,
+ nfp_dump_specific_level);
+ if (err)
+ return err;
+
/* Set size of actual dump, to trigger warning if different from
* calculated size.
*/
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 05/10] nfp: dump rtsyms
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Support rtsym TLVs.
- If specified rtsym is not found, dump the spec TLV as -ENOENT error.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 124 +++++++++++++++++++++
1 file changed, 124 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index d52e01ca6621..746e114a8da8 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -43,6 +43,7 @@
#define ALIGN8(x) ALIGN(x, 8)
enum nfp_dumpspec_type {
+ NFP_DUMPSPEC_TYPE_RTSYM = 4,
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
NFP_DUMPSPEC_TYPE_ERROR = 10001,
};
@@ -59,6 +60,34 @@ struct nfp_dump_tl {
char data[0];
};
+/* NFP CPP parameters */
+struct nfp_dumpspec_cpp_isl_id {
+ u8 target;
+ u8 action;
+ u8 token;
+ u8 island;
+};
+
+struct nfp_dump_common_cpp {
+ struct nfp_dumpspec_cpp_isl_id cpp_id;
+ __be32 offset; /* address to start dump */
+ __be32 dump_length; /* total bytes to dump, aligned to reg size */
+};
+
+struct nfp_dumpspec_rtsym {
+ struct nfp_dump_tl tl;
+ char rtsym[0];
+};
+
+struct nfp_dump_rtsym {
+ struct nfp_dump_tl tl;
+ struct nfp_dump_common_cpp cpp;
+ __be32 error; /* error code encountered while reading */
+ u8 padded_name_length; /* pad so data starts at 8 byte boundary */
+ char rtsym[0];
+ /* after padded_name_length, there is dump_length data */
+};
+
struct nfp_dump_prolog {
struct nfp_dump_tl tl;
__be32 dump_level;
@@ -122,6 +151,12 @@ nfp_traverse_tlvs(struct nfp_pf *pf, void *data, u32 data_length, void *param,
return 0;
}
+static u32 nfp_get_numeric_cpp_id(struct nfp_dumpspec_cpp_isl_id *cpp_id)
+{
+ return NFP_CPP_ISLAND_ID(cpp_id->target, cpp_id->action, cpp_id->token,
+ cpp_id->island);
+}
+
struct nfp_dumpspec *
nfp_net_dump_load_dumpspec(struct nfp_cpp *cpp, struct nfp_rtsym_table *rtbl)
{
@@ -162,11 +197,36 @@ static int nfp_dump_error_tlv_size(struct nfp_dump_tl *spec)
}
static int
+nfp_calc_rtsym_dump_sz(struct nfp_pf *pf, struct nfp_dump_tl *spec)
+{
+ struct nfp_rtsym_table *rtbl = pf->rtbl;
+ struct nfp_dumpspec_rtsym *spec_rtsym;
+ const struct nfp_rtsym *sym;
+ u32 tl_len, key_len;
+
+ spec_rtsym = (struct nfp_dumpspec_rtsym *)spec;
+ tl_len = be32_to_cpu(spec->length);
+ key_len = strnlen(spec_rtsym->rtsym, tl_len);
+ if (key_len == tl_len)
+ return nfp_dump_error_tlv_size(spec);
+
+ sym = nfp_rtsym_lookup(rtbl, spec_rtsym->rtsym);
+ if (!sym)
+ return nfp_dump_error_tlv_size(spec);
+
+ return ALIGN8(offsetof(struct nfp_dump_rtsym, rtsym) + key_len + 1) +
+ ALIGN8(sym->size);
+}
+
+static int
nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
u32 *size = param;
switch (be32_to_cpu(tl->type)) {
+ case NFP_DUMPSPEC_TYPE_RTSYM:
+ *size += nfp_calc_rtsym_dump_sz(pf, tl);
+ break;
default:
*size += nfp_dump_error_tlv_size(tl);
break;
@@ -247,12 +307,76 @@ nfp_dump_error_tlv(struct nfp_dump_tl *spec, int error,
}
static int
+nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_rtsym *dump_header = dump->p;
+ struct nfp_dumpspec_cpp_isl_id cpp_params;
+ struct nfp_rtsym_table *rtbl = pf->rtbl;
+ const struct nfp_rtsym *sym;
+ u32 header_size, total_size;
+ u32 tl_len, key_len;
+ int bytes_read;
+ u32 cpp_id;
+ void *dest;
+ int err;
+
+ tl_len = be32_to_cpu(spec->tl.length);
+ key_len = strnlen(spec->rtsym, tl_len);
+ if (key_len == tl_len)
+ return nfp_dump_error_tlv(&spec->tl, -EINVAL, dump);
+
+ sym = nfp_rtsym_lookup(rtbl, spec->rtsym);
+ if (!sym)
+ return nfp_dump_error_tlv(&spec->tl, -ENOENT, dump);
+
+ header_size =
+ ALIGN8(offsetof(struct nfp_dump_rtsym, rtsym) + key_len + 1);
+ total_size = header_size + ALIGN8(sym->size);
+ dest = dump->p + header_size;
+
+ err = nfp_add_tlv(be32_to_cpu(spec->tl.type), total_size, dump);
+ if (err)
+ return err;
+
+ dump_header->padded_name_length =
+ header_size - offsetof(struct nfp_dump_rtsym, rtsym);
+ memcpy(dump_header->rtsym, spec->rtsym, key_len + 1);
+
+ cpp_params.target = sym->target;
+ cpp_params.action = NFP_CPP_ACTION_RW;
+ cpp_params.token = 0;
+ cpp_params.island = sym->domain;
+ cpp_id = nfp_get_numeric_cpp_id(&cpp_params);
+
+ dump_header->cpp.cpp_id = cpp_params;
+ dump_header->cpp.offset = cpu_to_be32(sym->addr);
+ dump_header->cpp.dump_length = cpu_to_be32(sym->size);
+
+ bytes_read = nfp_cpp_read(pf->cpp, cpp_id, sym->addr, dest, sym->size);
+ if (bytes_read != sym->size) {
+ if (bytes_read >= 0)
+ bytes_read = -EIO;
+ dump_header->error = cpu_to_be32(bytes_read);
+ }
+
+ return 0;
+}
+
+static int
nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
+ struct nfp_dumpspec_rtsym *spec_rtsym;
struct nfp_dump_state *dump = param;
int err;
switch (be32_to_cpu(tl->type)) {
+ case NFP_DUMPSPEC_TYPE_RTSYM:
+ spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
+ err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);
+ if (err)
+ return err;
+ break;
default:
err = nfp_dump_error_tlv(tl, -EOPNOTSUPP, dump);
if (err)
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 06/10] nfp: dump all hwinfo
From: Simon Horman @ 2017-12-04 22:34 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: netdev, oss-drivers, Carl Heymann, Simon Horman
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Carl Heymann <carl.heymann@netronome.com>
- Dump hwinfo as separate TLV chunk, in a packed format containing
zero-separated key and value strings.
- This provides additional debug context, if requested by the dumpspec.
Signed-off-by: Carl Heymann <carl.heymann@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
.../net/ethernet/netronome/nfp/nfp_net_debugdump.c | 33 ++++++++++++++++++++++
drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h | 2 ++
.../ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c | 10 +++++++
3 files changed, 45 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
index 746e114a8da8..449cf0a3a158 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
@@ -44,6 +44,7 @@
enum nfp_dumpspec_type {
NFP_DUMPSPEC_TYPE_RTSYM = 4,
+ NFP_DUMPSPEC_TYPE_HWINFO = 5,
NFP_DUMPSPEC_TYPE_PROLOG = 10000,
NFP_DUMPSPEC_TYPE_ERROR = 10001,
};
@@ -222,11 +223,16 @@ static int
nfp_add_tlv_size(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
{
u32 *size = param;
+ u32 hwinfo_size;
switch (be32_to_cpu(tl->type)) {
case NFP_DUMPSPEC_TYPE_RTSYM:
*size += nfp_calc_rtsym_dump_sz(pf, tl);
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO:
+ hwinfo_size = nfp_hwinfo_get_packed_str_size(pf->hwinfo);
+ *size += sizeof(struct nfp_dump_tl) + ALIGN8(hwinfo_size);
+ break;
default:
*size += nfp_dump_error_tlv_size(tl);
break;
@@ -307,6 +313,28 @@ nfp_dump_error_tlv(struct nfp_dump_tl *spec, int error,
}
static int
+nfp_dump_hwinfo(struct nfp_pf *pf, struct nfp_dump_tl *spec,
+ struct nfp_dump_state *dump)
+{
+ struct nfp_dump_tl *dump_header = dump->p;
+ u32 hwinfo_size, total_size;
+ char *hwinfo;
+ int err;
+
+ hwinfo = nfp_hwinfo_get_packed_strings(pf->hwinfo);
+ hwinfo_size = nfp_hwinfo_get_packed_str_size(pf->hwinfo);
+ total_size = sizeof(*dump_header) + ALIGN8(hwinfo_size);
+
+ err = nfp_add_tlv(NFP_DUMPSPEC_TYPE_HWINFO, total_size, dump);
+ if (err)
+ return err;
+
+ memcpy(dump_header->data, hwinfo, hwinfo_size);
+
+ return 0;
+}
+
+static int
nfp_dump_single_rtsym(struct nfp_pf *pf, struct nfp_dumpspec_rtsym *spec,
struct nfp_dump_state *dump)
{
@@ -377,6 +405,11 @@ nfp_dump_for_tlv(struct nfp_pf *pf, struct nfp_dump_tl *tl, void *param)
if (err)
return err;
break;
+ case NFP_DUMPSPEC_TYPE_HWINFO:
+ err = nfp_dump_hwinfo(pf, tl, dump);
+ if (err)
+ return err;
+ break;
default:
err = nfp_dump_error_tlv(tl, -EOPNOTSUPP, dump);
if (err)
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
index 3ce51f03126f..ced62d112aa2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
@@ -49,6 +49,8 @@
struct nfp_hwinfo;
struct nfp_hwinfo *nfp_hwinfo_read(struct nfp_cpp *cpp);
const char *nfp_hwinfo_lookup(struct nfp_hwinfo *hwinfo, const char *lookup);
+char *nfp_hwinfo_get_packed_strings(struct nfp_hwinfo *hwinfo);
+u32 nfp_hwinfo_get_packed_str_size(struct nfp_hwinfo *hwinfo);
/* Implemented in nfp_nsp.c, low level functions */
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
index 4f24aff1e772..063a9a6243d6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_hwinfo.c
@@ -302,3 +302,13 @@ const char *nfp_hwinfo_lookup(struct nfp_hwinfo *hwinfo, const char *lookup)
return NULL;
}
+
+char *nfp_hwinfo_get_packed_strings(struct nfp_hwinfo *hwinfo)
+{
+ return hwinfo->data;
+}
+
+u32 nfp_hwinfo_get_packed_str_size(struct nfp_hwinfo *hwinfo)
+{
+ return le32_to_cpu(hwinfo->size) - sizeof(u32);
+}
--
2.11.0
^ permalink raw reply related
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