* [PATCH net-next 3/4] net: dsa: bcm_sf2: Advertise number of egress queues
From: Florian Fainelli @ 2017-09-01 23:59 UTC (permalink / raw)
To: netdev
Cc: davem, andrew, vivien.didelot, jiri, jhs, xiyou.wangcong,
Florian Fainelli
In-Reply-To: <1504310354-42773-1-git-send-email-f.fainelli@gmail.com>
The switch supports 8 egress queues per port, so indicate that such that
net/dsa/slave.c::dsa_slave_create can allocate the right number of TX
queues.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 8492c9d64004..3f1ad9d5d7c5 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1147,6 +1147,9 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
ds = dev->ds;
ds->ops = &bcm_sf2_ops;
+ /* Advertise the 8 egress queues */
+ ds->num_tx_queues = 8;
+
dev_set_drvdata(&pdev->dev, priv);
spin_lock_init(&priv->indir_lock);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: dsa: bcm_sf2: Configure IMP port TC2QOS mapping
From: Florian Fainelli @ 2017-09-01 23:59 UTC (permalink / raw)
To: netdev
Cc: davem, andrew, vivien.didelot, jiri, jhs, xiyou.wangcong,
Florian Fainelli
In-Reply-To: <1504310354-42773-1-git-send-email-f.fainelli@gmail.com>
Even though TC2QOS mapping is for switch egress queues, we need to
configure it correclty in order for the Broadcom tag ingress (CPU ->
switch) queue selection to work correctly since there is a 1:1 mapping
between switch egress queues and ingress queues.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 3f1ad9d5d7c5..fc9f9f171e55 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -103,6 +103,7 @@ static void bcm_sf2_brcm_hdr_setup(struct bcm_sf2_priv *priv, int port)
static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+ unsigned int i;
u32 reg, offset;
if (priv->type == BCM7445_DEVICE_ID)
@@ -129,6 +130,14 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
reg |= MII_DUMB_FWDG_EN;
core_writel(priv, reg, CORE_SWITCH_CTRL);
+ /* Configure Traffic Class to QoS mapping, allow each priority to map
+ * to a different queue number
+ */
+ reg = core_readl(priv, CORE_PORT_TC2_QOS_MAP_PORT(port));
+ for (i = 0; i < 8; i++)
+ reg |= i << (PRT_TO_QID_SHIFT * i);
+ core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
+
bcm_sf2_brcm_hdr_setup(priv, port);
/* Force link status for IMP port */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] net: phy: broadcom: force master mode for BCM54210E and B50212E
From: Florian Fainelli @ 2017-09-02 0:22 UTC (permalink / raw)
To: Rafał Miłecki, David S . Miller, netdev
Cc: Andrew Lunn, Hauke Mehrtens, bcm-kernel-feedback-list,
Rafał Miłecki, andrew
In-Reply-To: <20170901092121.4596-1-zajec5@gmail.com>
On 09/01/2017 02:21 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> First of all let me explain that the code we use for BCM54210E is also
> executed for the B50212E. They are very similar so it probably makes
> sense but it may be worth noting. The IDs are:
> 0x600d84a1: BCM54210E (rev B0)
> 0x600d84a2: BCM54210E (rev B1)
> 0x600d84a5: B50212E (rev B0)
> 0x600d84a6: B50212E (rev B1)
>
> I got a report that a board with BCM47189 SoC and B50212E B1 PHY doesn't
> work well with Intel's I217-LM and I218-LM:
> http://ark.intel.com/products/60019/Intel-Ethernet-Connection-I217-LM
> http://ark.intel.com/products/71307/Intel-Ethernet-Connection-I218-LM
> I was told there are massive ping loss.
>
> A solution to this problem is setting master mode in the 1000BASE-T
> register. I noticed a similar fix is present in the tg3 driver. One
> thing I'm not sure if this is needed for BCM54210E. It shouldn't hurt
> however since both are so similar.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> David: I'm not 100% sure if this is the best fix, so let's give others
> (Florian?) a moment to look at it / review it, please.
> ---
> drivers/net/phy/broadcom.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 1e9ad30a35c8..2569db0923b0 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -43,6 +43,10 @@ static int bcm54210e_config_init(struct phy_device *phydev)
> val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
> bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
>
> + val = phy_read(phydev, MII_CTRL1000);
> + val |= CTL1000_AS_MASTER | CTL1000_ENABLE_MASTER;
> + phy_write(phydev, MII_CTRL1000, val);
So for both BCM54210E and BCM50212E, the default values are to have
CTL1000_AS_MASTER cleared, which means that the PHY is configured as a
slave, and CTRL1000_ENABLE_MASTER also clear, which means Automatic
Slave/Master configuration, which is a bit confusing.
I would be more comfortable if you introduced a new flag after
PHY_BRCM_DIS_TXCRXC_NOENRGY in order to configure these bits or not.
Your driver (bgmac I suppose?) could then set this flag at phy_connect()
time through phydev->dev_flags.
Chances are that you are not breaking other set ups, because I suspect
we might be the offender here but it might be better to limit that to
just the devices you have.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] inetpeer: fix RCU lookup()
From: David Miller @ 2017-09-02 0:33 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1504299812.15310.26.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 01 Sep 2017 14:03:32 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Excess of seafood or something happened while I cooked the commit
> adding RB tree to inetpeer.
>
> Of course, RCU rules need to be respected or bad things can happen.
>
> In this particular loop, we need to read *pp once per iteration, not
> twice.
>
> Fixes: b145425f269a ("inetpeer: remove AVL implementation in favor of RB tree")
> Reported-by: John Sperbeck <jsperbeck@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Cheers for excess seafood :-)
Applied.
^ permalink raw reply
* Re: [PATCH] net: ethernet: ibm-emac: Add 5482 PHY init for OpenBlocks 600
From: Florian Fainelli @ 2017-09-02 0:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt, netdev
In-Reply-To: <1504241089.4974.67.camel@kernel.crashing.org>
On 08/31/2017 09:44 PM, Benjamin Herrenschmidt wrote:
> The vendor patches initialize those registers to get the
> PHY working properly.
>
> Sadly I don't have that PHY spec and whatever Broadcom PHY
> code we already have don't seem to document these two shadow
> registers (unless I miscalculated the address) so I'm keeping
> this as "vendor magic for that board". The vendor has long
> abandoned that product, but I find it handy to test ppc405
> kernels and so would like to keep it alive upstream :-)
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Note: Ideally, the whole driver should switch over to the
> generic PHY layer. However this is a much bigger undertaking
> which requires access to a bunch of HW to test, and for which
> I have neither the time nor the HW available these days.
Yes it sure does and the function names are so close, it is almost
irresistible not to do it.
>
> (Some of the HW could prove hard to find ...)
> ---
> drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/ethernet/ibm/emac/phy.c b/drivers/net/ethernet/ibm/emac/phy.c
> index 35865d05fccd..daa10de542fb 100644
> --- a/drivers/net/ethernet/ibm/emac/phy.c
> +++ b/drivers/net/ethernet/ibm/emac/phy.c
> @@ -24,6 +24,7 @@
> #include <linux/mii.h>
> #include <linux/ethtool.h>
> #include <linux/delay.h>
> +#include <linux/of.h>
>
> #include "emac.h"
> #include "phy.h"
> @@ -363,6 +364,34 @@ static struct mii_phy_def bcm5248_phy_def = {
> .ops = &generic_phy_ops
> };
>
> +static int bcm5482_init(struct mii_phy *phy)
> +{
> + if (!of_machine_is_compatible("plathome,obs600"))
> + return 0;
You can probably include brcmphy.h and pull the definition for at least
0x1c: MII_BCM54XX_SHD
> +
> + /* Magic inits from vendor original patches */
> + phy_write(phy, 0x1c, 0xa410);
What you are doing here is write to shadow register 9 (9 << 10) which is
the LED control register, and making the activity LED be driven on
activity/link as opposed to just activity. So this can probably be
written as:
phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE |
MII_BCM54XX_SHD_VAL(9) | MII_BCM54XX_SHD_DATA(BIT(4));
> + phy_write(phy, 0x1c, 0x8804);
And here you are writing to the spare control 1 register and setting bit
2 (which appears reserved but this is not clear) which would be enabling
the activity LED for 10BaseT or no link which can be written as:
phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE |
MII_BCM54XX_SHD_VAL(2) | MII_BCM4XX_SHD_DATA(BIT(2));
So basically you are touching registers that only affect LED
configuration and should not be doing anything else...
> +
> + return 0;
> +}
> +
> +static const struct mii_phy_ops bcm5482_phy_ops = {
> + .init = bcm5482_init,
> + .setup_aneg = genmii_setup_aneg,
> + .setup_forced = genmii_setup_forced,
> + .poll_link = genmii_poll_link,
> + .read_link = genmii_read_link
> +};
> +
> +static struct mii_phy_def bcm5482_phy_def = {
> +
> + .phy_id = 0x0143bcb0,
> + .phy_id_mask = 0x0ffffff0,
> + .name = "BCM5482 Gigabit Ethernet",
> + .ops = &bcm5482_phy_ops
> +};
> +
> static int m88e1111_init(struct mii_phy *phy)
> {
> pr_debug("%s: Marvell 88E1111 Ethernet\n", __func__);
> @@ -499,6 +528,7 @@ static struct mii_phy_def *mii_phy_table[] = {
> &et1011c_phy_def,
> &cis8201_phy_def,
> &bcm5248_phy_def,
> + &bcm5482_phy_def,
> &m88e1111_phy_def,
> &m88e1112_phy_def,
> &ar8035_phy_def,
>
--
Florian
^ permalink raw reply
* [PATCH net-next] net: systemport: Correctly set TSB endian for host
From: Florian Fainelli @ 2017-09-02 0:32 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Similarly to how we configure the RSB (Receive Status Block) we also
need to set the TSB (Transmit Status Block) based on the host endian.
This was missing from the commit indicated below.
Fixes: 389a06bc534e ("net: systemport: Set correct RSB endian bits based on host")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 13 +++++++++++++
drivers/net/ethernet/broadcom/bcmsysport.h | 3 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 931751e4f369..ef13b6041ef1 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1390,6 +1390,19 @@ static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
tdma_writel(priv, RING_IGNORE_STATUS, TDMA_DESC_RING_MAPPING(index));
tdma_writel(priv, 0, TDMA_DESC_RING_PCP_DEI_VID(index));
+ /* Do not use tdma_control_bit() here because TSB_SWAP1 collides
+ * with the original definition of ACB_ALGO
+ */
+ reg = tdma_readl(priv, TDMA_CONTROL);
+ if (priv->is_lite)
+ reg &= ~BIT(TSB_SWAP1);
+ /* Set a correct TSB format based on host endian */
+ if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+ reg |= tdma_control_bit(priv, TSB_SWAP0);
+ else
+ reg &= ~tdma_control_bit(priv, TSB_SWAP0);
+ tdma_writel(priv, reg, TDMA_CONTROL);
+
/* Program the number of descriptors as MAX_THRESHOLD and half of
* its size for the hysteresis trigger
*/
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 80b4ffff63b7..82e401df199e 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -449,7 +449,8 @@ struct bcm_rsb {
/* Uses 2 bits on SYSTEMPORT Lite and shifts everything by 1 bit, we
* keep the SYSTEMPORT layout here and adjust with tdma_control_bit()
*/
-#define TSB_SWAP 2
+#define TSB_SWAP0 2
+#define TSB_SWAP1 3
#define ACB_ALGO 3
#define BUF_DATA_OFFSET_SHIFT 4
#define BUF_DATA_OFFSET_MASK 0x3ff
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] net: ethernet: ibm-emac: Add 5482 PHY init for OpenBlocks 600
From: Benjamin Herrenschmidt @ 2017-09-02 1:22 UTC (permalink / raw)
To: Florian Fainelli, netdev
In-Reply-To: <816adc32-9df6-3433-b30c-79490a6328d1@gmail.com>
On Fri, 2017-09-01 at 17:35 -0700, Florian Fainelli wrote:
> On 08/31/2017 09:44 PM, Benjamin Herrenschmidt wrote:
> > The vendor patches initialize those registers to get the
> > PHY working properly.
> >
> > Sadly I don't have that PHY spec and whatever Broadcom PHY
> > code we already have don't seem to document these two shadow
> > registers (unless I miscalculated the address) so I'm keeping
> > this as "vendor magic for that board". The vendor has long
> > abandoned that product, but I find it handy to test ppc405
> > kernels and so would like to keep it alive upstream :-)
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >
> > Note: Ideally, the whole driver should switch over to the
> > generic PHY layer. However this is a much bigger undertaking
> > which requires access to a bunch of HW to test, and for which
> > I have neither the time nor the HW available these days.
>
> Yes it sure does and the function names are so close, it is almost
> irresistible not to do it.
I think there's some common ancestry :-)
That said, I'm weary of doing it without proper testing, especially
those old cell blades which I'm not sure I still have a functional
one, and whatever is using gpcs...
Cheers,
Ben.
>
> >
> > (Some of the HW could prove hard to find ...)
> > ---
> > drivers/net/ethernet/ibm/emac/phy.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/ibm/emac/phy.c b/drivers/net/ethernet/ibm/emac/phy.c
> > index 35865d05fccd..daa10de542fb 100644
> > --- a/drivers/net/ethernet/ibm/emac/phy.c
> > +++ b/drivers/net/ethernet/ibm/emac/phy.c
> > @@ -24,6 +24,7 @@
> > #include <linux/mii.h>
> > #include <linux/ethtool.h>
> > #include <linux/delay.h>
> > +#include <linux/of.h>
> >
> > #include "emac.h"
> > #include "phy.h"
> > @@ -363,6 +364,34 @@ static struct mii_phy_def bcm5248_phy_def = {
> > .ops = &generic_phy_ops
> > };
> >
> > +static int bcm5482_init(struct mii_phy *phy)
> > +{
> > + if (!of_machine_is_compatible("plathome,obs600"))
> > + return 0;
>
> You can probably include brcmphy.h and pull the definition for at least
> 0x1c: MII_BCM54XX_SHD
Yup.
> > +
> > + /* Magic inits from vendor original patches */
> > + phy_write(phy, 0x1c, 0xa410);
>
> What you are doing here is write to shadow register 9 (9 << 10) which is
> the LED control register, and making the activity LED be driven on
> activity/link as opposed to just activity. So this can probably be
> written as:
Ok so I really don't *need* that in fact.
> phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE |
> MII_BCM54XX_SHD_VAL(9) | MII_BCM54XX_SHD_DATA(BIT(4));
>
> > + phy_write(phy, 0x1c, 0x8804);
>
> And here you are writing to the spare control 1 register and setting bit
> 2 (which appears reserved but this is not clear) which would be enabling
> the activity LED for 10BaseT or no link which can be written as:
>
> phy_write(phy, MII_BCM54XX_SHD, MII_BCM54XX_SHD_WRITE |
> MII_BCM54XX_SHD_VAL(2) | MII_BCM4XX_SHD_DATA(BIT(2));
>
> So basically you are touching registers that only affect LED
> configuration and should not be doing anything else...
I wonder if I need to bother at all then. I was worried it was related
to actual function of the device, but if it's just LEDs, I think I may
as well just drop it.
> > +
> > + return 0;
> > +}
> > +
> > +static const struct mii_phy_ops bcm5482_phy_ops = {
> > + .init = bcm5482_init,
> > + .setup_aneg = genmii_setup_aneg,
> > + .setup_forced = genmii_setup_forced,
> > + .poll_link = genmii_poll_link,
> > + .read_link = genmii_read_link
> > +};
> > +
> > +static struct mii_phy_def bcm5482_phy_def = {
> > +
> > + .phy_id = 0x0143bcb0,
> > + .phy_id_mask = 0x0ffffff0,
> > + .name = "BCM5482 Gigabit Ethernet",
> > + .ops = &bcm5482_phy_ops
> > +};
> > +
> > static int m88e1111_init(struct mii_phy *phy)
> > {
> > pr_debug("%s: Marvell 88E1111 Ethernet\n", __func__);
> > @@ -499,6 +528,7 @@ static struct mii_phy_def *mii_phy_table[] = {
> > &et1011c_phy_def,
> > &cis8201_phy_def,
> > &bcm5248_phy_def,
> > + &bcm5482_phy_def,
> > &m88e1111_phy_def,
> > &m88e1112_phy_def,
> > &ar8035_phy_def,
> >
>
>
^ permalink raw reply
* Re: [PATCH net-next v6] net: stmmac: Delete dead code for MDIO registration
From: David Miller @ 2017-09-02 1:30 UTC (permalink / raw)
To: romain.perier
Cc: peppe.cavallaro, alexandre.torgue, andrew, f.fainelli, netdev,
linux-kernel
In-Reply-To: <20170831135303.12381-1-romain.perier@collabora.com>
From: Romain Perier <romain.perier@collabora.com>
Date: Thu, 31 Aug 2017 15:53:03 +0200
> This code is no longer used, the logging function was changed by commit
> fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register").
> It was previously showing information about the type of the IRQ, if it's
> polled, ignored or a normal interrupt. As we don't want information loss,
> I have moved this code to phy_attached_print().
>
> Fixes: fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register")
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
You'll need to respin this against net-next as phy_attached_print() has had
some changes recently.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: phy: bcm7xxx: make array bcm7xxx_suspend_cfg static, reduces object code size
From: David Miller @ 2017-09-02 1:31 UTC (permalink / raw)
To: colin.king; +Cc: andrew, f.fainelli, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170831135715.27978-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 31 Aug 2017 14:57:15 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate the array bcm7xxx_suspend_cfg A on the stack, instead
> make it static. Makes the object code smaller by over 300 bytes:
>
> Before:
> text data bss dec hex filename
> 6351 8146 0 14497 38a1 drivers/net/phy/bcm7xxx.o
>
> After:
> text data bss dec hex filename
> 5986 8210 0 14196 3774 drivers/net/phy/bcm7xxx.o
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH][net-next] net: qualcomm: rmnet: remove unused variable priv
From: David Miller @ 2017-09-02 1:34 UTC (permalink / raw)
To: colin.king; +Cc: subashab, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170831140727.28170-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 31 Aug 2017 15:07:27 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> priv is being assigned but is never used, so remove it.
>
> Cleans up clang build warning:
> "warning: Value stored to 'priv' is never read"
>
> Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ipv6: sr: Use ARRAY_SIZE macro
From: David Miller @ 2017-09-02 1:35 UTC (permalink / raw)
To: thomas; +Cc: netdev, linux-kernel
In-Reply-To: <20170831141815.28003-1-thomas@m3y3r.de>
From: Thomas Meyer <thomas@m3y3r.de>
Date: Thu, 31 Aug 2017 16:18:15 +0200
> Grepping for "sizeof\(.+\) / sizeof\(" found this as one of the first
> candidates.
> Maybe a coccinelle can catch all of those.
>
> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v5 0/2] report TCP MD5 signing keys and addresses
From: David Miller @ 2017-09-02 1:38 UTC (permalink / raw)
To: colona; +Cc: eric.dumazet, netdev
In-Reply-To: <20170831165939.5121-1-colona@arista.com>
From: Ivan Delalande <colona@arista.com>
Date: Thu, 31 Aug 2017 09:59:37 -0700
> Allow userspace to retrieve MD5 signature keys and addresses configured
> on TCP sockets through inet_diag.
...
Series applied to net-next, thanks.
^ permalink raw reply
* Re: netdev carrier changes is one even after ethernet link up.
From: Florian Fainelli @ 2017-09-02 1:55 UTC (permalink / raw)
To: Bhadram Varka, andrew@lunn.ch; +Cc: linux-netdev
In-Reply-To: <cd860399cf9148d193a08636a6d82a68@DRBGMAIL102.nvidia.com>
On 08/31/2017 10:49 PM, Bhadram Varka wrote:
> Thanks for responding. Now responding inline
>
>> -----Original Message-----
>> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
>> Sent: Friday, September 01, 2017 5:53 AM
>> To: Bhadram Varka <vbhadram@nvidia.com>; andrew@lunn.ch
>> Cc: linux-netdev <netdev@vger.kernel.org>
>> Subject: Re: netdev carrier changes is one even after ethernet link up.
>>
>> On 08/30/2017 10:53 PM, Bhadram Varka wrote:
>>> Hi,
>>>
>>>
>>>
>>> I have observed that carrier_changes is one even in case of the
>>> ethernet link is up.
>>>
>>>
>>>
>>> After investigating the code below is my observation –
>>>
>>>
>>>
>>> ethernet_driver_probe()
>>>
>>> +--->phy_connect()
>>>
>>> | +--->phy_attach_direct()
>>>
>>> | +---> netif_carrier_off() : which increments
>>> carrier_changes to one.
>>>
>>> +--->register_netdevice() : will the carrier_changes becomes zero here ?
>>>
>>> +--->netif_carrier_off(): not increment the carrier_changes since
>>> __LINK_STATE_NOCARRIER already set.
>>>
>>>
>>>
>>> From ethernet driver open will start the PHY and trigger the
>>> phy_state_machine.
>>>
>>> Phy_state_machine workqueue calling netif_carrier_on() once the link is
>> UP.
>>>
>>> netif_carrier_on() increments the carrier_changes by one.
>>
>> If the call trace is correct, then there is at least two problems here:
>>
>> - phy_connect() does start the PHY machine which means that as soon as it
>> detects a link state of any kind (up or down) it can call
>> netif_carrier_off() respectively netif_carrier_on()
>>
>> - as soon as you call register_netdevice() notifiers run and other parts of the
>> kernel or user-space programs can see an inconsistent link state
>>
>> I would suggest doing the following sequence instead:
>>
>> netif_carrier_off()
>> register_netdevice()
>> phy_connect()
>>
>> Which should result in a consistent link state and carrier value.
>>
> Yes, It will address the issue.
>
> If we did the phy_conect in ndo_open it will make the carrier changes as two. But if we did in probe function then it's not working.
>
> In ethernet driver probe - (below sequence is not working)
> phy_connect()
> register_netdevice()
> netif_carrier_off()
>
> working sequence:
> In probe():
> register_netdevice()
> ndo_open:
> phy_connect()
>
> After reverting - https://lkml.org/lkml/2016/1/9/173 this works if we do phy_connect in probe as well.
But as mentioned before you should not be doing the PHY probe in your
driver's probe function for different reasons:
- the probe function's responsibility is to initialize the driver and
the HW to a state where they both have everything needed but it should
be in quiesced state. There is no guarantee that your network device may
ever be used after probe unless something calls ndo_open(), you should
therefore keep all resources to a minimum: memory allocated, HW powered
down etc.
- there is a race condition between the PHY state machine started in
phy_connect(), and when register_netdevice() is called and notifiers
running which can lead to an inconsistent state for the carrier
So considering that your driver does not do that, I am not sure what you
are expecting...
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] net: systemport: Correctly set TSB endian for host
From: David Miller @ 2017-09-02 3:19 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <1504312354-62020-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 1 Sep 2017 17:32:34 -0700
> Similarly to how we configure the RSB (Receive Status Block) we also
> need to set the TSB (Transmit Status Block) based on the host endian.
> This was missing from the commit indicated below.
>
> Fixes: 389a06bc534e ("net: systemport: Set correct RSB endian bits based on host")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks Florian.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] net: ubuf_info.refcnt conversion
From: David Miller @ 2017-09-02 3:22 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, willemb
In-Reply-To: <1504287389.15310.21.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 01 Sep 2017 10:36:29 -0700
> On Thu, 2017-08-31 at 17:04 -0700, Eric Dumazet wrote:
>> On Thu, 2017-08-31 at 16:48 -0700, Eric Dumazet wrote:
>> > Yet another atomic_t -> refcount_t conversion, split in two patches.
>> >
>> > First patch prepares the automatic conversion done in the second patch.
>> >
>> > Eric Dumazet (2):
>> > net: prepare (struct ubuf_info)->refcnt conversion
>> > net: convert (struct ubuf_info)->refcnt to refcount_t
>> >
>> > drivers/vhost/net.c | 2 +-
>> > include/linux/skbuff.h | 5 +++--
>> > net/core/skbuff.c | 14 ++++----------
>> > net/ipv4/tcp.c | 2 --
>> > 4 files changed, 8 insertions(+), 15 deletions(-)
>> >
>>
>> David please ignore this series, I will send a V3 :)
>>
>
> No need for a V3, sorry for the confusion, but we had to double check
> with Willem that everything had been covered.
>
> Please tell me if I need to resend, thanks !
Ok, series applied, thanks Eric.
^ permalink raw reply
* Re: [net-next PATCH] bpf: sockmap update/simplify memory accounting scheme
From: David Miller @ 2017-09-02 3:29 UTC (permalink / raw)
To: john.fastabend; +Cc: netdev, daniel, ast
In-Reply-To: <20170901182926.8981.77450.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 01 Sep 2017 11:29:26 -0700
> Instead of tracking wmem_queued and sk_mem_charge by incrementing
> in the verdict SK_REDIRECT paths and decrementing in the tx work
> path use skb_set_owner_w and sock_writeable helpers. This solves
> a few issues with the current code. First, in SK_REDIRECT inc on
> sk_wmem_queued and sk_mem_charge were being done without the peers
> sock lock being held. Under stress this can result in accounting
> errors when tx work and/or multiple verdict decisions are working
> on the peer psock.
>
> Additionally, this cleans up the code because we can rely on the
> default destructor to decrement memory accounting on kfree_skb. Also
> this will trigger sk_write_space when space becomes available on
> kfree_skb() which wasn't happening before and prevent __sk_free
> from being called until all in-flight packets are completed.
>
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/2] netvsc: transparent VF related cleanups
From: David Miller @ 2017-09-02 3:31 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20170831231613.15805-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 31 Aug 2017 16:16:11 -0700
> The first gets rid of unnecessary ref counting, and second
> allows removing hv_netvsc driver even if VF present.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next] net: Add module reference to FIB notifiers
From: David Miller @ 2017-09-02 3:34 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, dsahern, mlxsw
In-Reply-To: <20170901091517.31271-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Fri, 1 Sep 2017 12:15:17 +0300
> When a listener registers to the FIB notification chain it receives a
> dump of the FIB entries and rules from existing address families by
> invoking their dump operations.
>
> While we call into these modules we need to make sure they aren't
> removed. Do that by increasing their reference count before invoking
> their dump operations and decrease it afterwards.
>
> Fixes: 04b1d4e50e82 ("net: core: Make the FIB notification chain generic")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Oops, yes, you'll need to do this.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next, 0/4] cleanups and fixes of channel settings
From: David Miller @ 2017-09-02 3:39 UTC (permalink / raw)
To: haiyangz, haiyangz; +Cc: netdev, kys, olaf, vkuznets, linux-kernel
In-Reply-To: <20170901213007.10325-1-haiyangz@exchange.microsoft.com>
From: Haiyang Zhang <haiyangz@exchange.microsoft.com>
Date: Fri, 1 Sep 2017 14:30:03 -0700
> This patch set cleans up some unused variables, unnecessary checks.
> Also fixed some limit checking of channel number.
Series applied.
^ permalink raw reply
* Re: [PATCH] ipv6: sr: Use ARRAY_SIZE macro
From: Joe Perches @ 2017-09-02 3:51 UTC (permalink / raw)
To: David Miller, thomas; +Cc: netdev, linux-kernel
In-Reply-To: <20170901.183540.2023957151661748902.davem@davemloft.net>
On Fri, 2017-09-01 at 18:35 -0700, David Miller wrote:
> From: Thomas Meyer <thomas@m3y3r.de>
> Date: Thu, 31 Aug 2017 16:18:15 +0200
>
> > Grepping for "sizeof\(.+\) / sizeof\(" found this as one of the first
> > candidates.
> > Maybe a coccinelle can catch all of those.
Umm: try scripts/coccinelle/misc/array_size.cocci
Until then, maybe a perl script?
$ git grep --name-only sizeof.*/.*sizeof drivers/net | \
xargs perl -p -i -e 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\/\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)/ARRAY_SIZE(\1)/g'
gives:
$ git diff --stat drivers/net
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/fw.c | 4 +--
drivers/net/ethernet/mellanox/mlx4/main.c | 8 +++---
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phytbl_n.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------------------
5 files changed, 101 insertions(+), 101 deletions(-)
^ permalink raw reply
* [PATCH v2 net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Yonghong Song @ 2017-09-02 5:48 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170902054824.371962-1-yhs@fb.com>
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch adds helper bpf_perf_read_counter_time for kprobed based perf
event array map, to read perf counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
To achieve scaling factor between two bpf invocations, users
can can use cpu_id as the key (which is typical for perf array usage model)
to remember the previous value and do the calculation inside the
bpf program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 3 ++-
include/uapi/linux/bpf.h | 21 ++++++++++++++++++++-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/verifier.c | 4 +++-
kernel/events/core.c | 19 +++++++++++++------
kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++----
6 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index b14095b..5a50808 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -898,7 +898,8 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr,
void *context);
extern void perf_pmu_migrate_context(struct pmu *pmu,
int src_cpu, int dst_cpu);
-int perf_event_read_local(struct perf_event *event, u64 *value);
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running);
extern u64 perf_event_read_value(struct perf_event *event,
u64 *enabled, u64 *running);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba848b7..9c23bef 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -582,6 +582,14 @@ union bpf_attr {
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
+ *
+ * int bpf_perf_read_counter_time(map, flags, counter_time_buf, buf_size)
+ * read perf event counter value and perf event enabled/running time
+ * @map: pointer to perf_event_array map
+ * @flags: index of event in the map or bitmask flags
+ * @counter_time_buf: buf to fill
+ * @buf_size: size of the counter_time_buf
+ * Return: 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -638,6 +646,7 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
+ FN(perf_read_counter_time), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -681,7 +690,8 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_read_counter_time flags. */
#define BPF_F_INDEX_MASK 0xffffffffULL
#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
/* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -864,4 +874,13 @@ enum {
#define TCP_BPF_IW 1001 /* Set TCP initial congestion window */
#define TCP_BPF_SNDCWND_CLAMP 1002 /* Set sndcwnd_clamp */
+struct bpf_perf_time {
+ __u64 enabled;
+ __u64 running;
+};
+struct bpf_perf_counter_time {
+ __u64 counter;
+ struct bpf_perf_time time;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 98c0f00..68d8666 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -492,7 +492,7 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map,
ee = ERR_PTR(-EOPNOTSUPP);
event = perf_file->private_data;
- if (perf_event_read_local(event, &value) == -EOPNOTSUPP)
+ if (perf_event_read_local(event, &value, NULL, NULL) == -EOPNOTSUPP)
goto err_out;
ee = bpf_event_entry_gen(perf_file, map_file);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..c4d29e3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1494,7 +1494,8 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
if (func_id != BPF_FUNC_perf_event_read &&
- func_id != BPF_FUNC_perf_event_output)
+ func_id != BPF_FUNC_perf_event_output &&
+ func_id != BPF_FUNC_perf_read_counter_time)
goto error;
break;
case BPF_MAP_TYPE_STACK_TRACE:
@@ -1537,6 +1538,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_FUNC_perf_event_read:
case BPF_FUNC_perf_event_output:
+ case BPF_FUNC_perf_read_counter_time:
if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
goto error;
break;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8c01572..20c4039 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3670,7 +3670,8 @@ static inline u64 perf_event_count(struct perf_event *event)
* will not be local and we cannot read them atomically
* - must not have a pmu::count method
*/
-int perf_event_read_local(struct perf_event *event, u64 *value)
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running)
{
unsigned long flags;
int ret = 0;
@@ -3694,7 +3695,7 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
* It must not have a pmu::count method, those are not
* NMI safe.
*/
- if (event->pmu->count) {
+ if (value && event->pmu->count) {
ret = -EOPNOTSUPP;
goto out;
}
@@ -3718,10 +3719,16 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
* or local to this CPU. Furthermore it means its ACTIVE (otherwise
* oncpu == -1).
*/
- if (event->oncpu == smp_processor_id())
- event->pmu->read(event);
-
- *value = local64_read(&event->count);
+ if (value) {
+ if (event->oncpu == smp_processor_id())
+ event->pmu->read(event);
+ *value = local64_read(&event->count);
+ }
+ if (enabled && running) {
+ u64 ctx_time = event->shadow_ctx_time + perf_clock();
+ *enabled = ctx_time - event->tstamp_enabled;
+ *running = ctx_time - event->tstamp_running;
+ }
out:
local_irq_restore(flags);
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dc498b6..7ef953f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
return &bpf_trace_printk_proto;
}
-BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
-{
+static __always_inline int
+get_map_perf_counter(struct bpf_map *map, u64 flags,
+ u64 *value, u64 *enabled, u64 *running) {
struct bpf_array *array = container_of(map, struct bpf_array, map);
unsigned int cpu = smp_processor_id();
u64 index = flags & BPF_F_INDEX_MASK;
struct bpf_event_entry *ee;
- u64 value = 0;
int err;
if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
@@ -275,7 +275,17 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
if (!ee)
return -ENOENT;
- err = perf_event_read_local(ee->event, &value);
+ err = perf_event_read_local(ee->event, value, enabled, running);
+ return err;
+}
+
+
+BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
+{
+ u64 value = 0;
+ int err;
+
+ err = get_map_perf_counter(map, flags, &value, NULL, NULL);
/*
* this api is ugly since we miss [-22..-2] range of valid
* counter values, but that's uapi
@@ -285,6 +295,20 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
return value;
}
+BPF_CALL_4(bpf_perf_read_counter_time, struct bpf_map *, map, u64, flags,
+ struct bpf_perf_counter_time *, buf, u32, size)
+{
+ int err;
+
+ if (unlikely(size != sizeof(struct bpf_perf_counter_time)))
+ return -EINVAL;
+ err = get_map_perf_counter(map, flags, &buf->counter, &buf->time.enabled,
+ &buf->time.running);
+ if (err)
+ return err;
+ return 0;
+}
+
static const struct bpf_func_proto bpf_perf_event_read_proto = {
.func = bpf_perf_event_read,
.gpl_only = true,
@@ -293,6 +317,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
.arg2_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_read_counter_time_proto = {
+ .func = bpf_perf_read_counter_time,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg4_type = ARG_CONST_SIZE,
+};
+
static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
static __always_inline u64
@@ -499,6 +533,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_perf_event_output_proto;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto;
+ case BPF_FUNC_perf_read_counter_time:
+ return &bpf_perf_read_counter_time_proto;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 3/4] bpf: add helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-02 5:48 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170902054824.371962-1-yhs@fb.com>
This patch adds helper bpf_perf_prog_read_time for perf event based bpf
programs, to read event enabled/running time.
The enabled/running time is accumulated since the perf event open.
The typical use case for perf event based bpf program is to attach itself
to a single event. In such cases, if it is desirable to get scaling factor
between two bpf invocations, users can can save the time values in a map,
and use the value from the map and the current value to calculate
the scaling factor.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 1 +
include/uapi/linux/bpf.h | 8 ++++++++
kernel/events/core.c | 1 +
kernel/trace/bpf_trace.c | 23 +++++++++++++++++++++++
4 files changed, 33 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 5a50808..6756ae7 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -821,6 +821,7 @@ struct perf_output_handle {
struct bpf_perf_event_data_kern {
struct pt_regs *regs;
struct perf_sample_data *data;
+ struct perf_event *event;
};
#ifdef CONFIG_CGROUP_PERF
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9c23bef..1ae55c8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -590,6 +590,13 @@ union bpf_attr {
* @counter_time_buf: buf to fill
* @buf_size: size of the counter_time_buf
* Return: 0 on success or negative error code
+ *
+ * int bpf_perf_prog_read_time(ctx, time_buf, buf_size)
+ * Read perf event enabled and running time
+ * @ctx: pointer to ctx
+ * @time_buf: buf to fill
+ * @buf_size: size of the time_buf
+ * Return : 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -647,6 +654,7 @@ union bpf_attr {
FN(sk_redirect_map), \
FN(sock_map_update), \
FN(perf_read_counter_time), \
+ FN(perf_prog_read_time), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 20c4039..338f564 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8026,6 +8026,7 @@ static void bpf_overflow_handler(struct perf_event *event,
struct bpf_perf_event_data_kern ctx = {
.data = data,
.regs = regs,
+ .event = event,
};
int ret = 0;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 7ef953f..89b0744 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -603,6 +603,18 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
flags, 0, 0);
}
+BPF_CALL_3(bpf_perf_prog_read_time_tp, void *, ctx, struct bpf_perf_time *,
+ time_buf, u32, size)
+{
+ struct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;
+
+ if (size != sizeof(struct bpf_perf_time))
+ return -EINVAL;
+
+ return perf_event_read_local(kctx->event, NULL, &time_buf->enabled,
+ &time_buf->running);
+}
+
static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.func = bpf_get_stackid_tp,
.gpl_only = true,
@@ -612,6 +624,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.arg3_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_prog_read_time_proto_tp = {
+ .func = bpf_perf_prog_read_time_tp,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg3_type = ARG_CONST_SIZE,
+};
+
static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
@@ -619,6 +640,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
return &bpf_perf_event_output_proto_tp;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto_tp;
+ case BPF_FUNC_perf_prog_read_time:
+ return &bpf_perf_prog_read_time_proto_tp;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 0/4] bpf: add two helpers to read perf event enabled/running time
From: Yonghong Song @ 2017-09-02 5:48 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch set implements two helper functions.
The helper bpf_perf_read_counter_time reads counter/time_enabled/time_running
for perf event array map. The helper bpf_perf_prog_read_time read
time_enabled/time_running for bpf prog with type BPF_PROG_TYPE_PERF_EVENT.
Yonghong Song (4):
bpf: add helper bpf_perf_read_counter_time for perf event array map
bpf: add a test case to read enabled/running time for perf array
bpf: add helper bpf_perf_prog_read_time
bpf: add a test case for helper bpf_perf_prog_read_time
include/linux/perf_event.h | 4 +-
include/uapi/linux/bpf.h | 29 ++++++++++++-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/verifier.c | 4 +-
kernel/events/core.c | 20 ++++++---
kernel/trace/bpf_trace.c | 67 +++++++++++++++++++++++++++++--
samples/bpf/trace_event_kern.c | 10 +++++
samples/bpf/trace_event_user.c | 13 +++---
samples/bpf/tracex6_kern.c | 26 ++++++++++++
samples/bpf/tracex6_user.c | 13 +++++-
tools/testing/selftests/bpf/bpf_helpers.h | 7 ++++
11 files changed, 175 insertions(+), 20 deletions(-)
--
2.9.5
^ permalink raw reply
* [PATCH v2 net-next 4/4] bpf: add a test case for helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-02 5:48 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170902054824.371962-1-yhs@fb.com>
The bpf sample program trace_event is enhanced to use the new
helper to print out enabled/running time.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/trace_event_kern.c | 10 ++++++++++
samples/bpf/trace_event_user.c | 13 ++++++++-----
tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 41b6115..e93c8b1 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -37,10 +37,14 @@ struct bpf_map_def SEC("maps") stackmap = {
SEC("perf_event")
int bpf_prog1(struct bpf_perf_event_data *ctx)
{
+ char time_fmt1[] = "Time Enabled: %llu, Time Running: %llu";
+ char time_fmt2[] = "Get Time Failed, ErrCode: %d";
char fmt[] = "CPU-%d period %lld ip %llx";
u32 cpu = bpf_get_smp_processor_id();
+ struct bpf_perf_time time_buf;
struct key_t key;
u64 *val, one = 1;
+ int ret;
if (ctx->sample_period < 10000)
/* ignore warmup */
@@ -54,6 +58,12 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
return 0;
}
+ ret = bpf_perf_prog_read_time(ctx, (void *)&time_buf, sizeof(struct bpf_perf_time));
+ if (!ret)
+ bpf_trace_printk(time_fmt1, sizeof(time_fmt1), time_buf.enabled, time_buf.running);
+ else
+ bpf_trace_printk(time_fmt2, sizeof(time_fmt2), ret);
+
val = bpf_map_lookup_elem(&counts, &key);
if (val)
(*val)++;
diff --git a/samples/bpf/trace_event_user.c b/samples/bpf/trace_event_user.c
index 7bd827b..bf4f1b6 100644
--- a/samples/bpf/trace_event_user.c
+++ b/samples/bpf/trace_event_user.c
@@ -127,6 +127,9 @@ static void test_perf_event_all_cpu(struct perf_event_attr *attr)
int *pmu_fd = malloc(nr_cpus * sizeof(int));
int i, error = 0;
+ /* system wide perf event, no need to inherit */
+ attr->inherit = 0;
+
/* open perf_event on all cpus */
for (i = 0; i < nr_cpus; i++) {
pmu_fd[i] = sys_perf_event_open(attr, -1, i, -1, 0);
@@ -154,6 +157,11 @@ static void test_perf_event_task(struct perf_event_attr *attr)
{
int pmu_fd;
+ /* per task perf event, enable inherit so the "dd ..." command can be traced properly.
+ * Enabling inherit will cause bpf_perf_prog_read_time helper failure.
+ */
+ attr->inherit = 1;
+
/* open task bound event */
pmu_fd = sys_perf_event_open(attr, 0, -1, -1, 0);
if (pmu_fd < 0) {
@@ -175,14 +183,12 @@ static void test_bpf_perf_event(void)
.freq = 1,
.type = PERF_TYPE_HARDWARE,
.config = PERF_COUNT_HW_CPU_CYCLES,
- .inherit = 1,
};
struct perf_event_attr attr_type_sw = {
.sample_freq = SAMPLE_FREQ,
.freq = 1,
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_l1d = {
.sample_freq = SAMPLE_FREQ,
@@ -192,7 +198,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_L1D |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_branch_miss = {
.sample_freq = SAMPLE_FREQ,
@@ -202,7 +207,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_BPU |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_type_raw = {
.sample_freq = SAMPLE_FREQ,
@@ -210,7 +214,6 @@ static void test_bpf_perf_event(void)
.type = PERF_TYPE_RAW,
/* Intel Instruction Retired */
.config = 0xc0,
- .inherit = 1,
};
printf("Test HW_CPU_CYCLES\n");
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index fe41852..ddad690 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -74,6 +74,9 @@ static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
void *counter_time_buf,
unsigned int buf_size) =
(void *) BPF_FUNC_perf_read_counter_time;
+static int (*bpf_perf_prog_read_time)(void *ctx, void *time_buf,
+ unsigned int size) =
+ (void *) BPF_FUNC_perf_prog_read_time;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* [PATCH v2 net-next 2/4] bpf: add a test case to read enabled/running time for perf array
From: Yonghong Song @ 2017-09-02 5:48 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170902054824.371962-1-yhs@fb.com>
The bpf sample program tracex6 is enhanced to use the new
helper to read enabled/running time as well.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/tracex6_kern.c | 26 ++++++++++++++++++++++++++
samples/bpf/tracex6_user.c | 13 ++++++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 4 ++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/tracex6_kern.c b/samples/bpf/tracex6_kern.c
index e7d1803..46acfef 100644
--- a/samples/bpf/tracex6_kern.c
+++ b/samples/bpf/tracex6_kern.c
@@ -15,6 +15,12 @@ struct bpf_map_def SEC("maps") values = {
.value_size = sizeof(u64),
.max_entries = 64,
};
+struct bpf_map_def SEC("maps") values2 = {
+ .type = BPF_MAP_TYPE_HASH,
+ .key_size = sizeof(int),
+ .value_size = sizeof(struct bpf_perf_counter_time),
+ .max_entries = 64,
+};
SEC("kprobe/htab_map_get_next_key")
int bpf_prog1(struct pt_regs *ctx)
@@ -37,5 +43,25 @@ int bpf_prog1(struct pt_regs *ctx)
return 0;
}
+SEC("kprobe/htab_map_lookup_elem")
+int bpf_prog2(struct pt_regs *ctx)
+{
+ u32 key = bpf_get_smp_processor_id();
+ struct bpf_perf_counter_time *val, buf;
+ int error;
+
+ error = bpf_perf_read_counter_time(&counters, key, &buf, sizeof(buf));
+ if (error)
+ return 0;
+
+ val = bpf_map_lookup_elem(&values2, &key);
+ if (val)
+ *val = buf;
+ else
+ bpf_map_update_elem(&values2, &key, &buf, BPF_NOEXIST);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/tracex6_user.c b/samples/bpf/tracex6_user.c
index a05a99a..2a0c5d8 100644
--- a/samples/bpf/tracex6_user.c
+++ b/samples/bpf/tracex6_user.c
@@ -22,6 +22,7 @@
static void check_on_cpu(int cpu, struct perf_event_attr *attr)
{
+ struct bpf_perf_counter_time value2;
int pmu_fd, error = 0;
cpu_set_t set;
__u64 value;
@@ -46,8 +47,18 @@ static void check_on_cpu(int cpu, struct perf_event_attr *attr)
fprintf(stderr, "Value missing for CPU %d\n", cpu);
error = 1;
goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: %llu\n", cpu, value);
+ }
+ /* The above bpf_map_lookup_elem should trigger the second kprobe */
+ if (bpf_map_lookup_elem(map_fd[2], &cpu, &value2)) {
+ fprintf(stderr, "Value2 missing for CPU %d\n", cpu);
+ error = 1;
+ goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: counter: %llu, enabled: %llu, running: %llu\n", cpu,
+ value2.counter, value2.time.enabled, value2.time.running);
}
- fprintf(stderr, "CPU %d: %llu\n", cpu, value);
on_exit:
assert(bpf_map_delete_elem(map_fd[0], &cpu) == 0 || error);
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 36fb916..fe41852 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -70,6 +70,10 @@ static int (*bpf_sk_redirect_map)(void *map, int key, int flags) =
static int (*bpf_sock_map_update)(void *map, void *key, void *value,
unsigned long long flags) =
(void *) BPF_FUNC_sock_map_update;
+static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
+ void *counter_time_buf,
+ unsigned int buf_size) =
+ (void *) BPF_FUNC_perf_read_counter_time;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ 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