* Re: [PATCH RFC 0/2] Add support for warnings to extack
From: Marcelo Ricardo Leitner @ 2018-03-18 18:20 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <5f894da5-6e45-c70c-e061-e7572a4390f1@gmail.com>
On Sun, Mar 18, 2018 at 10:11:52AM -0600, David Ahern wrote:
> On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> > Currently we have the limitation that warnings cannot be reported though
> > extack. For example, when tc flower failed to get offloaded but got
> > installed on software datapath. The hardware failure is not fatal and
> > thus extack is not even shared with the driver, so the error is simply
> > omitted from any logging.
>
> If this set ends up moving forward, the above statement needs to be
> corrected: extack allows non-error messages to be sent back to the user,
> so the above must be talking about some other limitation local to tc.
Right.
Thanks,
Marcelo
^ permalink raw reply
* [PATCH net-next] net: dsa: mv88e6xxx: Fix missing register lock in serdes_get_stats
From: Florian Fainelli @ 2018-03-18 18:23 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, open list
We can hit the register lock not held assertion with the following path:
[ 34.170631] mv88e6085 0.1:00: Switch registers lock not held!
[ 34.176510] CPU: 0 PID: 950 Comm: ethtool Not tainted 4.16.0-rc4 #143
[ 34.182985] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
[ 34.189519] Backtrace:
[ 34.192033] [<8010c4b4>] (dump_backtrace) from [<8010c788>] (show_stack+0x20/0x24)
[ 34.199680] r6:9f5dc010 r5:00000011 r4:9f5dc010 r3:00000000
[ 34.205434] [<8010c768>] (show_stack) from [<80679d38>] (dump_stack+0x24/0x28)
[ 34.212719] [<80679d14>] (dump_stack) from [<804844a8>] (mv88e6xxx_read+0x70/0x7c)
[ 34.220376] [<80484438>] (mv88e6xxx_read) from [<804870dc>] (mv88e6xxx_port_get_cmode+0x34/0x4c)
[ 34.229257] r5:a09cd128 r4:9ee31d07
[ 34.232880] [<804870a8>] (mv88e6xxx_port_get_cmode) from [<80487e6c>] (mv88e6352_port_has_serdes+0x24/0x64)
[ 34.242690] r4:9f5dc010
[ 34.245309] [<80487e48>] (mv88e6352_port_has_serdes) from [<804880b8>] (mv88e6352_serdes_get_stats+0x28/0x12c)
[ 34.255389] r4:00000001
[ 34.257973] [<80488090>] (mv88e6352_serdes_get_stats) from [<804811e8>] (mv88e6xxx_get_ethtool_stats+0xb0/0xc0)
[ 34.268156] r10:00000000 r9:00000000 r8:00000000 r7:a09cd020 r6:00000001 r5:9f5dc01c
[ 34.276052] r4:9f5dc010
[ 34.278631] [<80481138>] (mv88e6xxx_get_ethtool_stats) from [<8064f740>] (dsa_slave_get_ethtool_stats+0xbc/0xc4)
mv88e6xxx_get_ethtool_stats() calls mv88e6xxx_get_stats() which calls both
chip->info->ops->stats_get_stats(), which holds the register lock, and
chip->info->ops->serdes_get_stats() which does not. Have
chip->info->ops->serdes_get_stats() be running with the register lock held to
avoid such assertions.
Fixes: 436fe17d273b ("net: dsa: mv88e6xxx: Allow the SERDES interfaces to have statistics")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index bd3ee84770c7..c22c18ed2de3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -849,7 +849,9 @@ static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
if (chip->info->ops->serdes_get_stats) {
data += count;
+ mutex_lock(&chip->reg_lock);
chip->info->ops->serdes_get_stats(chip, port, data);
+ mutex_unlock(&chip->reg_lock);
}
}
--
2.14.1
^ permalink raw reply related
* Re: [PATCH RFC 0/2] Add support for warnings to extack
From: Marcelo Ricardo Leitner @ 2018-03-18 18:36 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <5f894da5-6e45-c70c-e061-e7572a4390f1@gmail.com>
On Sun, Mar 18, 2018 at 10:11:52AM -0600, David Ahern wrote:
> On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> > Currently we have the limitation that warnings cannot be reported though
> > extack. For example, when tc flower failed to get offloaded but got
> > installed on software datapath. The hardware failure is not fatal and
> > thus extack is not even shared with the driver, so the error is simply
> > omitted from any logging.
>
> If this set ends up moving forward, the above statement needs to be
> corrected: extack allows non-error messages to be sent back to the user,
> so the above must be talking about some other limitation local to tc.
I'll split this patchset into two:
- pass extack to drivers when doing tc offload (such as flower and
others) and allow reporting of warnings in such cases. (according to
a opt-in flag, as discussed in the other subthread)
- allow more than one message
The 1st may lead to the 2nd but right now it's more as a supposition,
as there is no actual user for it yet.
^ permalink raw reply
* [PATCH 1/2] brcmfmac: add new dt entries for SG SDIO settings
From: Alexey Roslyakov @ 2018-03-18 18:41 UTC (permalink / raw)
To: kvalo, robh+dt, mark.rutland, arend.vanspriel, franky.lin,
hante.meuleman, chi-hsien.lin, wright.feng, netdev
Cc: linux-wireless, devicetree, linux-kernel, brcm80211-dev-list.pdl,
brcm80211-dev-list, Alexey Roslyakov
There are 3 fields in SDIO settings (quirks) to workaround some of
the SG SDIO host particularities, i.e higher align requirements for
SG items.
All coding is done the long time ago, but there is no way to change the
driver behavior without patching the kernel.
Add missing devicetree entries.
Signed-off-by: Alexey Roslyakov <alexey.roslyakov@gmail.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index aee6e5937c41..0718ca09a40d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -31,6 +31,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
int irq;
u32 irqf;
u32 val;
+ u16 align;
if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
!of_device_is_compatible(np, "brcm,bcm4329-fmac"))
@@ -39,6 +40,15 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
sdio->drive_strength = val;
+ sdio->broken_sg_support =
+ of_property_read_bool(np, "brcm,broken-sg-support");
+
+ if (of_property_read_u16(np, "brcm,sd-head-align", &align) == 0)
+ sdio->sd_head_align = align;
+
+ if (of_property_read_u16(np, "brcm,sd-sgentry-align", &align) == 0)
+ sdio->sd_sgentry_align = align;
+
/* make sure there are interrupts defined in the node */
if (!of_find_property(np, "interrupts", NULL))
return;
--
2.16.1
^ permalink raw reply related
* [PATCH 2/2] dt: bindings: add new dt entries for brcmfmac
From: Alexey Roslyakov @ 2018-03-18 18:41 UTC (permalink / raw)
To: kvalo, robh+dt, mark.rutland, arend.vanspriel, franky.lin,
hante.meuleman, chi-hsien.lin, wright.feng, netdev
Cc: linux-wireless, devicetree, linux-kernel, brcm80211-dev-list.pdl,
brcm80211-dev-list, Alexey Roslyakov
In-Reply-To: <20180318184101.26951-1-alexey.roslyakov@gmail.com>
In case if the host has higher align requirements for SG items, allow
setting device-specific aligns for scatterlist items.
Signed-off-by: Alexey Roslyakov <alexey.roslyakov@gmail.com>
---
Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
index 86602f264dce..187b8c1b52a7 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -17,6 +17,11 @@ Optional properties:
When not specified the device will use in-band SDIO interrupts.
- interrupt-names : name of the out-of-band interrupt, which must be set
to "host-wake".
+ - brcm,broken-sg-support : boolean flag to indicate that the SDIO host
+ controller has higher align requirement than 32 bytes for each
+ scatterlist item.
+ - brcm,sd-head-align : alignment requirement for start of data buffer.
+ - brcm,sd-sgentry-align : length alignment requirement for each sg entry.
Example:
--
2.16.1
^ permalink raw reply related
* Re: [PATCH v2 10/21] lightnvm: Remove depends on HAS_DMA in case of platform dependency
From: Matias Bjørling @ 2018-03-18 18:46 UTC (permalink / raw)
To: Geert Uytterhoeven, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Felipe Balbi, Greg Kroah-Hartman,
James E . J . Bottomley, Martin K . Petersen, Andrew Morton,
Mark Brown, Liam Girdwood, Tejun Heo, Herbert Xu,
David S . Miller, Bartlomiej Zolnierkiewicz, Stefan Richter,
Alan Tull, Moritz Fischer, Wolfram Sang
Cc: iommu, linux-usb, linux-scsi, alsa-devel, linux-ide, linux-crypto,
linux-fbdev, linux1394-devel, linux-fpga, linux-i2c, linux-iio,
linux-block, linux-media, linux-mmc, linux-mtd, netdev,
linux-remoteproc, linux-serial, linux-spi, devel, linux-kernel
In-Reply-To: <1521208314-4783-11-git-send-email-geert@linux-m68k.org>
On 03/16/2018 02:51 PM, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
>
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
>
> This simplifies the dependencies, and allows to improve compile-testing.
>
> Notes:
> - FSL_FMAN keeps its dependency on HAS_DMA, as it calls set_dma_ops(),
> which does not exist if HAS_DMA=n (Do we need a dummy? The use of
> set_dma_ops() in this driver is questionable),
> - SND_SOC_LPASS_IPQ806X and SND_SOC_LPASS_PLATFORM loose their
> dependency on HAS_DMA, as they are selected from
> SND_SOC_APQ8016_SBC.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> ---
> v2:
> - Add Reviewed-by, Acked-by,
> - Drop RFC state,
> - Split per subsystem.
> ---
> drivers/lightnvm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/lightnvm/Kconfig b/drivers/lightnvm/Kconfig
> index 10c08982185a572f..9c03f35d9df113c6 100644
> --- a/drivers/lightnvm/Kconfig
> +++ b/drivers/lightnvm/Kconfig
> @@ -4,7 +4,7 @@
>
> menuconfig NVM
> bool "Open-Channel SSD target support"
> - depends on BLOCK && HAS_DMA && PCI
> + depends on BLOCK && PCI
> select BLK_DEV_NVME
> help
> Say Y here to get to enable Open-channel SSDs.
>
Looks good.
Reviewed-by: Matias Bjørling <mb@lightnvm.io>
^ permalink raw reply
* [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support
From: Florian Fainelli @ 2018-03-18 18:52 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, privat, andrew, vivien.didelot, davem,
rmk+kernel, sean.wang, Woojung.Huh, john, cphealy
Hi all,
This patch series adds PHYLINK support to DSA which is necessary to support more
complex PHY and pluggable modules setups.
Patch series can be found here:
https://github.com/ffainelli/linux/commits/dsa-phylink
This was tested on:
- dsa-loop
- bcm_sf2
- mv88e6xxx
- b53
With a variety of test cases:
- internal & external MDIO PHYs
- MoCA with link notification through interrupt/MMIO register
- built-in PHYs
- ifconfig up/down for several cycles works
- bind/unbind of the drivers
And everything should still work as expected. Please be aware of the following:
- switch drivers (like bcm_sf2) which may have user-facing network ports using
fixed links would need to implement phylink_mac_ops to remain functional.
PHYLINK does not create a phy_device for fixed links, therefore our
call to adjust_link() from phylink_mac_link_{up,down} would not be calling
into the driver. This *should not* affect CPU/DSA ports which are configured
through adjust_link() but have no network devices
- support for SFP/SFF is now possible, but switch drivers will still need some
modifications to properly support those, including, but not limited to using
the correct binding information. This will be submitted on top of this series
Russell, we could theoretically eliminate patch 3 and resolve this within DSA
entirely by keeping a per-port phy_interface_t (we did that before), this is
not a big change if we have to, let me know if you feel like this is cleaner. I
was initially considering passing a phylink_link_state reference to
mac_link_{up,down} but only a couple of fields are valid during link_down and
ended up with passing the phy_interface_t value we need instead. This is
necessary for switch drivers which have different types of port interfaces (see
bcm_sf2 documentation in tree).
Thank you!
Florian Fainelli (4):
net: dsa: Eliminate dsa_slave_get_link()
net: phy: phylink: Provide PHY interface to mac_link_{up,down}
net: dsa: Plug in PHYLINK support
net: dsa: bcm_sf2: Implement phylink_mac_ops
drivers/net/dsa/bcm_sf2.c | 190 +++++++++++++--------
drivers/net/ethernet/marvell/mvneta.c | 4 +-
drivers/net/phy/phylink.c | 6 +-
include/linux/phylink.h | 10 +-
include/net/dsa.h | 27 ++-
net/dsa/Kconfig | 2 +-
net/dsa/dsa_priv.h | 9 -
net/dsa/slave.c | 304 ++++++++++++++++++++--------------
8 files changed, 340 insertions(+), 212 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net-next 1/4] net: dsa: Eliminate dsa_slave_get_link()
From: Florian Fainelli @ 2018-03-18 18:52 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, privat, andrew, vivien.didelot, davem,
rmk+kernel, sean.wang, Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-1-f.fainelli@gmail.com>
Since we use PHYLIB to manage the per-port link indication, this will
also be reflected correctly in the network device's carrier state, so we
can use ethtool_op_get_link() instead.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/slave.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 18561af7a8f1..9714e8b002d3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -498,16 +498,6 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
ds->ops->get_regs(ds, dp->index, regs, _p);
}
-static u32 dsa_slave_get_link(struct net_device *dev)
-{
- if (!dev->phydev)
- return -ENODEV;
-
- genphy_update_link(dev->phydev);
-
- return dev->phydev->link;
-}
-
static int dsa_slave_get_eeprom_len(struct net_device *dev)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -981,7 +971,7 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_regs_len = dsa_slave_get_regs_len,
.get_regs = dsa_slave_get_regs,
.nway_reset = phy_ethtool_nway_reset,
- .get_link = dsa_slave_get_link,
+ .get_link = ethtool_op_get_link,
.get_eeprom_len = dsa_slave_get_eeprom_len,
.get_eeprom = dsa_slave_get_eeprom,
.set_eeprom = dsa_slave_set_eeprom,
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: phy: phylink: Provide PHY interface to mac_link_{up,down}
From: Florian Fainelli @ 2018-03-18 18:52 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, privat, andrew, vivien.didelot, davem,
rmk+kernel, sean.wang, Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-1-f.fainelli@gmail.com>
In preparation for having DSA transition entirely to PHYLINK, we need to pass a
PHY interface type to the mac_link_{up,down} callbacks because we may have to
make decisions on that (e.g: turn on/off RGMII interfaces etc.). We do not pass
an entire phylink_link_state because not all parameters (pause, duplex etc.) are
defined when the link is down, only link and interface are.
Update mvneta accordingly since it currently implements phylink_mac_ops.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/marvell/mvneta.c | 4 +++-
drivers/net/phy/phylink.c | 6 +++++-
include/linux/phylink.h | 10 ++++++++--
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 25e9a551cc8c..60de9b8d62c2 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3396,7 +3396,8 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
}
-static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
+static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface)
{
struct mvneta_port *pp = netdev_priv(ndev);
u32 val;
@@ -3415,6 +3416,7 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
}
static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface,
struct phy_device *phy)
{
struct mvneta_port *pp = netdev_priv(ndev);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 51a011a349fe..cef3c1356a8c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -423,8 +423,10 @@ static void phylink_resolve(struct work_struct *w)
if (pl->phylink_disable_state) {
pl->mac_link_dropped = false;
link_state.link = false;
+ link_state.interface = pl->phy_state.interface;
} else if (pl->mac_link_dropped) {
link_state.link = false;
+ link_state.interface = pl->phy_state.interface;
} else {
switch (pl->link_an_mode) {
case MLO_AN_PHY:
@@ -470,10 +472,12 @@ static void phylink_resolve(struct work_struct *w)
if (link_state.link != netif_carrier_ok(ndev)) {
if (!link_state.link) {
netif_carrier_off(ndev);
- pl->ops->mac_link_down(ndev, pl->link_an_mode);
+ pl->ops->mac_link_down(ndev, pl->link_an_mode,
+ pl->phy_state.interface);
netdev_info(ndev, "Link is Down\n");
} else {
pl->ops->mac_link_up(ndev, pl->link_an_mode,
+ pl->phy_state.interface,
pl->phydev);
netif_carrier_on(ndev);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index bd137c273d38..f29a40947de9 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -73,8 +73,10 @@ struct phylink_mac_ops {
void (*mac_config)(struct net_device *ndev, unsigned int mode,
const struct phylink_link_state *state);
void (*mac_an_restart)(struct net_device *ndev);
- void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
+ void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface);
void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface,
struct phy_device *phy);
};
@@ -161,17 +163,20 @@ void mac_an_restart(struct net_device *ndev);
* mac_link_down() - take the link down
* @ndev: a pointer to a &struct net_device for the MAC.
* @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
*
* If @mode is not an in-band negotiation mode (as defined by
* phylink_autoneg_inband()), force the link down and disable any
* Energy Efficient Ethernet MAC configuration.
*/
-void mac_link_down(struct net_device *ndev, unsigned int mode);
+void mac_link_down(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface);
/**
* mac_link_up() - allow the link to come up
* @ndev: a pointer to a &struct net_device for the MAC.
* @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
* @phy: any attached phy
*
* If @mode is not an in-band negotiation mode (as defined by
@@ -180,6 +185,7 @@ void mac_link_down(struct net_device *ndev, unsigned int mode);
* phy_init_eee() and perform appropriate MAC configuration for EEE.
*/
void mac_link_up(struct net_device *ndev, unsigned int mode,
+ phy_interface_t interface,
struct phy_device *phy);
#endif
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support
From: Florian Fainelli @ 2018-03-18 18:52 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, privat, andrew, vivien.didelot, davem,
rmk+kernel, sean.wang, Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-1-f.fainelli@gmail.com>
Add support for PHYLINK within the DSA subsystem in order to support more
complex devices such as pluggable (SFP) and non-pluggable (SFF) modules, 10G
PHYs, and traditional PHYs. Using PHYLINK allows us to drop some amount of
complexity we had while probing fixed and non-fixed PHYs using Device Tree.
Because PHYLINK separates the Ethernet MAC/port configuration into different
stages, we let switch drivers implement those, and for now, we maintain
functionality by calling dsa_slave_adjust_link() during
phylink_mac_link_{up,down} which provides semantically equivalent steps.
Drivers willing to take advantage of PHYLINK should implement the phylink_mac_*
operations that DSA wraps.
We cannot quite remove the adjust_link() callback just yet, because a number of
drivers rely on that for configuring their "CPU" and "DSA" ports, this is done
dsa_port_setup_phy_of() and dsa_port_fixed_link_register_of() still.
Drivers that utilize fixed links for user-facing ports (e.g: bcm_sf2) will need
to implement phylink_mac_ops from now on to preserve functionality, since PHYLINK
*does not* create a phy_device instance for fixed links.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 12 +-
include/net/dsa.h | 27 ++++-
net/dsa/Kconfig | 2 +-
net/dsa/dsa_priv.h | 9 --
net/dsa/slave.c | 300 ++++++++++++++++++++++++++++------------------
5 files changed, 213 insertions(+), 137 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 0378eded31f2..726d75a61795 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -15,7 +15,7 @@
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
+#include <linux/phylink.h>
#include <linux/mii.h>
#include <linux/of.h>
#include <linux/of_irq.h>
@@ -563,7 +563,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
}
static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
- struct fixed_phy_status *status)
+ struct phylink_link_state *status)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
u32 duplex, pause, offset;
@@ -611,13 +611,11 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
core_writel(priv, reg, offset);
if ((pause & (1 << port)) &&
- (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT)))) {
- status->asym_pause = 1;
- status->pause = 1;
- }
+ (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT))))
+ status->pause |= MLO_PAUSE_TX;
if (pause & (1 << port))
- status->pause = 1;
+ status->pause |= MLO_PAUSE_TXRX_MASK;
}
static void bcm_sf2_enable_acb(struct dsa_switch *ds)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 60fb4ec8ba61..5399bed88df8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -20,12 +20,13 @@
#include <linux/of.h>
#include <linux/ethtool.h>
#include <linux/net_tstamp.h>
+#include <linux/phy.h>
#include <net/devlink.h>
#include <net/switchdev.h>
struct tc_action;
struct phy_device;
-struct fixed_phy_status;
+struct phylink_link_state;
enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = 0,
@@ -199,6 +200,7 @@ struct dsa_port {
u8 stp_state;
struct net_device *bridge_dev;
struct devlink_port devlink_port;
+ struct phylink *pl;
/*
* Original copy of the master netdev ethtool_ops
*/
@@ -350,8 +352,27 @@ struct dsa_switch_ops {
*/
void (*adjust_link)(struct dsa_switch *ds, int port,
struct phy_device *phydev);
+ /*
+ * PHYLINK integration
+ */
+ void (*phylink_validate)(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state);
+ int (*phylink_mac_link_state)(struct dsa_switch *ds, int port,
+ struct phylink_link_state *state);
+ void (*phylink_mac_config)(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ const struct phylink_link_state *state);
+ void (*phylink_mac_an_restart)(struct dsa_switch *ds, int port);
+ void (*phylink_mac_link_down)(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface);
+ void (*phylink_mac_link_up)(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface,
+ struct phy_device *phydev);
void (*fixed_link_update)(struct dsa_switch *ds, int port,
- struct fixed_phy_status *st);
+ struct phylink_link_state *state);
/*
* ethtool hardware statistics.
@@ -588,4 +609,6 @@ static inline int call_dsa_notifiers(unsigned long val, struct net_device *dev,
#define BRCM_TAG_GET_PORT(v) ((v) >> 8)
#define BRCM_TAG_GET_QUEUE(v) ((v) & 0xff)
+void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up);
+
#endif
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bbf2c82cf7b2..4183e4ba27a5 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -9,7 +9,7 @@ config NET_DSA
depends on HAVE_NET_DSA && MAY_USE_DEVLINK
depends on BRIDGE || BRIDGE=n
select NET_SWITCHDEV
- select PHYLIB
+ select PHYLINK
---help---
Say Y if you want to enable support for the hardware switches supported
by the Distributed Switch Architecture.
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 70de7895e5b8..aebaa5970d91 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -75,15 +75,6 @@ struct dsa_slave_priv {
/* DSA port data, such as switch, port index, etc. */
struct dsa_port *dp;
- /*
- * The phylib phy_device pointer for the PHY connected
- * to this port.
- */
- phy_interface_t phy_interface;
- int old_link;
- int old_pause;
- int old_duplex;
-
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *netpoll;
#endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9714e8b002d3..6d9914964c20 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -13,6 +13,7 @@
#include <linux/netdevice.h>
#include <linux/phy.h>
#include <linux/phy_fixed.h>
+#include <linux/phylink.h>
#include <linux/of_net.h>
#include <linux/of_mdio.h>
#include <linux/mdio.h>
@@ -97,8 +98,7 @@ static int dsa_slave_open(struct net_device *dev)
if (err)
goto clear_promisc;
- if (dev->phydev)
- phy_start(dev->phydev);
+ phylink_start(dp->pl);
return 0;
@@ -120,8 +120,7 @@ static int dsa_slave_close(struct net_device *dev)
struct net_device *master = dsa_slave_to_master(dev);
struct dsa_port *dp = dsa_slave_to_port(dev);
- if (dev->phydev)
- phy_stop(dev->phydev);
+ phylink_stop(dp->pl);
dsa_port_disable(dp, dev->phydev);
@@ -272,10 +271,7 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
break;
}
- if (!dev->phydev)
- return -ENODEV;
-
- return phy_mii_ioctl(dev->phydev, ifr, cmd);
+ return phylink_mii_ioctl(p->dp->pl, ifr, cmd);
}
static int dsa_slave_port_attr_set(struct net_device *dev,
@@ -498,6 +494,13 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
ds->ops->get_regs(ds, dp->index, regs, _p);
}
+static int dsa_slave_nway_reset(struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+
+ return phylink_ethtool_nway_reset(dp->pl);
+}
+
static int dsa_slave_get_eeprom_len(struct net_device *dev)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -536,6 +539,22 @@ static int dsa_slave_set_eeprom(struct net_device *dev,
return -EOPNOTSUPP;
}
+static int dsa_slave_get_module_info(struct net_device *dev,
+ struct ethtool_modinfo *modinfo)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+
+ return phylink_ethtool_get_module_info(dp->pl, modinfo);
+}
+
+static int dsa_slave_get_module_eeprom(struct net_device *dev,
+ struct ethtool_eeprom *ee, u8 *buf)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+
+ return phylink_ethtool_get_module_eeprom(dp->pl, ee, buf);
+}
+
static void dsa_slave_get_strings(struct net_device *dev,
uint32_t stringset, uint8_t *data)
{
@@ -608,6 +627,8 @@ static void dsa_slave_get_wol(struct net_device *dev, struct ethtool_wolinfo *w)
struct dsa_port *dp = dsa_slave_to_port(dev);
struct dsa_switch *ds = dp->ds;
+ phylink_ethtool_get_wol(dp->pl, w);
+
if (ds->ops->get_wol)
ds->ops->get_wol(ds, dp->index, w);
}
@@ -618,6 +639,8 @@ static int dsa_slave_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
struct dsa_switch *ds = dp->ds;
int ret = -EOPNOTSUPP;
+ phylink_ethtool_set_wol(dp->pl, w);
+
if (ds->ops->set_wol)
ret = ds->ops->set_wol(ds, dp->index, w);
@@ -641,13 +664,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
if (ret)
return ret;
- if (e->eee_enabled) {
- ret = phy_init_eee(dev->phydev, 0);
- if (ret)
- return ret;
- }
-
- return phy_ethtool_set_eee(dev->phydev, e);
+ return phylink_ethtool_set_eee(dp->pl, e);
}
static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
@@ -667,7 +684,23 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
if (ret)
return ret;
- return phy_ethtool_get_eee(dev->phydev, e);
+ return phylink_ethtool_get_eee(dp->pl, e);
+}
+
+static int dsa_slave_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+
+ return phylink_ethtool_ksettings_get(dp->pl, cmd);
+}
+
+static int dsa_slave_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+
+ return phylink_ethtool_ksettings_set(dp->pl, cmd);
}
#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -970,11 +1003,13 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_drvinfo = dsa_slave_get_drvinfo,
.get_regs_len = dsa_slave_get_regs_len,
.get_regs = dsa_slave_get_regs,
- .nway_reset = phy_ethtool_nway_reset,
+ .nway_reset = dsa_slave_nway_reset,
.get_link = ethtool_op_get_link,
.get_eeprom_len = dsa_slave_get_eeprom_len,
.get_eeprom = dsa_slave_get_eeprom,
.set_eeprom = dsa_slave_set_eeprom,
+ .get_module_info = dsa_slave_get_module_info,
+ .get_module_eeprom = dsa_slave_get_module_eeprom,
.get_strings = dsa_slave_get_strings,
.get_ethtool_stats = dsa_slave_get_ethtool_stats,
.get_sset_count = dsa_slave_get_sset_count,
@@ -982,8 +1017,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_wol = dsa_slave_get_wol,
.set_eee = dsa_slave_set_eee,
.get_eee = dsa_slave_get_eee,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_link_ksettings = dsa_slave_get_link_ksettings,
+ .set_link_ksettings = dsa_slave_set_link_ksettings,
.get_rxnfc = dsa_slave_get_rxnfc,
.set_rxnfc = dsa_slave_set_rxnfc,
.get_ts_info = dsa_slave_get_ts_info,
@@ -1042,56 +1077,120 @@ static struct device_type dsa_type = {
.name = "dsa",
};
-static void dsa_slave_adjust_link(struct net_device *dev)
+static void dsa_slave_phylink_validate(struct net_device *dev,
+ unsigned long *supported,
+ struct phylink_link_state *state)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
- struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = dp->ds;
- unsigned int status_changed = 0;
- if (p->old_link != dev->phydev->link) {
- status_changed = 1;
- p->old_link = dev->phydev->link;
- }
+ if (!ds->ops->phylink_validate)
+ return;
- if (p->old_duplex != dev->phydev->duplex) {
- status_changed = 1;
- p->old_duplex = dev->phydev->duplex;
- }
+ ds->ops->phylink_validate(ds, dp->index, supported, state);
+}
- if (p->old_pause != dev->phydev->pause) {
- status_changed = 1;
- p->old_pause = dev->phydev->pause;
- }
+static int dsa_slave_phylink_mac_link_state(struct net_device *dev,
+ struct phylink_link_state *state)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
- if (ds->ops->adjust_link && status_changed)
- ds->ops->adjust_link(ds, dp->index, dev->phydev);
+ /* Only called for SGMII and 802.3z */
+ if (!ds->ops->phylink_mac_link_state)
+ return -EOPNOTSUPP;
- if (status_changed)
- phy_print_status(dev->phydev);
+ return ds->ops->phylink_mac_link_state(ds, dp->index, state);
}
-static int dsa_slave_fixed_link_update(struct net_device *dev,
- struct fixed_phy_status *status)
+static void dsa_slave_phylink_mac_config(struct net_device *dev,
+ unsigned int mode,
+ const struct phylink_link_state *state)
{
- struct dsa_switch *ds;
- struct dsa_port *dp;
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
- if (dev) {
- dp = dsa_slave_to_port(dev);
- ds = dp->ds;
- if (ds->ops->fixed_link_update)
- ds->ops->fixed_link_update(ds, dp->index, status);
+ if (!ds->ops->phylink_mac_config)
+ return;
+
+ ds->ops->phylink_mac_config(ds, dp->index, mode, state);
+}
+
+static void dsa_slave_phylink_mac_an_restart(struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->phylink_mac_an_restart)
+ return;
+
+ ds->ops->phylink_mac_an_restart(ds, dp->index);
+}
+
+static void dsa_slave_phylink_mac_link_down(struct net_device *dev,
+ unsigned int mode,
+ phy_interface_t interface)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->phylink_mac_link_down) {
+ if (ds->ops->adjust_link && dev->phydev)
+ ds->ops->adjust_link(ds, dp->index, dev->phydev);
+ return;
}
- return 0;
+ ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface);
+}
+
+static void dsa_slave_phylink_mac_link_up(struct net_device *dev,
+ unsigned int mode,
+ phy_interface_t interface,
+ struct phy_device *phydev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->phylink_mac_link_up) {
+ if (ds->ops->adjust_link && dev->phydev)
+ ds->ops->adjust_link(ds, dp->index, dev->phydev);
+ return;
+ }
+
+ ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
+}
+
+static const struct phylink_mac_ops dsa_slave_phylink_mac_ops = {
+ .validate = dsa_slave_phylink_validate,
+ .mac_link_state = dsa_slave_phylink_mac_link_state,
+ .mac_config = dsa_slave_phylink_mac_config,
+ .mac_an_restart = dsa_slave_phylink_mac_an_restart,
+ .mac_link_down = dsa_slave_phylink_mac_link_down,
+ .mac_link_up = dsa_slave_phylink_mac_link_up,
+};
+
+void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up)
+{
+ const struct dsa_port *dp = dsa_to_port(ds, port);
+
+ phylink_mac_change(dp->pl, up);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_change);
+
+static void dsa_slave_phylink_fixed_state(struct net_device *dev,
+ struct phylink_link_state *state)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_switch *ds = dp->ds;
+
+ if (ds->ops->fixed_link_update)
+ ds->ops->fixed_link_update(ds, dp->index, state);
}
/* slave device setup *******************************************************/
static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
- struct dsa_slave_priv *p = netdev_priv(slave_dev);
struct dsa_switch *ds = dp->ds;
slave_dev->phydev = mdiobus_get_phy(ds->slave_mii_bus, addr);
@@ -1100,75 +1199,49 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
return -ENODEV;
}
- /* Use already configured phy mode */
- if (p->phy_interface == PHY_INTERFACE_MODE_NA)
- p->phy_interface = slave_dev->phydev->interface;
-
- return phy_connect_direct(slave_dev, slave_dev->phydev,
- dsa_slave_adjust_link, p->phy_interface);
+ return phylink_connect_phy(dp->pl, slave_dev->phydev);
}
static int dsa_slave_phy_setup(struct net_device *slave_dev)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
- struct dsa_slave_priv *p = netdev_priv(slave_dev);
struct device_node *port_dn = dp->dn;
struct dsa_switch *ds = dp->ds;
- struct device_node *phy_dn;
- bool phy_is_fixed = false;
u32 phy_flags = 0;
int mode, ret;
mode = of_get_phy_mode(port_dn);
if (mode < 0)
mode = PHY_INTERFACE_MODE_NA;
- p->phy_interface = mode;
- phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
- if (!phy_dn && of_phy_is_fixed_link(port_dn)) {
- /* In the case of a fixed PHY, the DT node associated
- * to the fixed PHY is the Port DT node
- */
- ret = of_phy_register_fixed_link(port_dn);
- if (ret) {
- netdev_err(slave_dev, "failed to register fixed PHY: %d\n", ret);
- return ret;
- }
- phy_is_fixed = true;
- phy_dn = of_node_get(port_dn);
+ dp->pl = phylink_create(slave_dev, of_fwnode_handle(port_dn), mode,
+ &dsa_slave_phylink_mac_ops);
+ if (IS_ERR(dp->pl)) {
+ netdev_err(slave_dev,
+ "error creating PHYLINK: %ld\n", PTR_ERR(dp->pl));
+ return PTR_ERR(dp->pl);
}
+ phylink_fixed_state_cb(dp->pl, dsa_slave_phylink_fixed_state);
+
if (ds->ops->get_phy_flags)
phy_flags = ds->ops->get_phy_flags(ds, dp->index);
- if (phy_dn) {
- slave_dev->phydev = of_phy_connect(slave_dev, phy_dn,
- dsa_slave_adjust_link,
- phy_flags,
- p->phy_interface);
- of_node_put(phy_dn);
- }
-
- if (slave_dev->phydev && phy_is_fixed)
- fixed_phy_set_link_update(slave_dev->phydev,
- dsa_slave_fixed_link_update);
-
- /* We could not connect to a designated PHY, so use the switch internal
- * MDIO bus instead
- */
- if (!slave_dev->phydev) {
+ ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
+ if (ret == -ENODEV) {
+ /* We could not connect to a designated PHY or SFP, so use the
+ * switch internal MDIO bus instead
+ */
ret = dsa_slave_phy_connect(slave_dev, dp->index);
if (ret) {
- netdev_err(slave_dev, "failed to connect to port %d: %d\n",
+ netdev_err(slave_dev,
+ "failed to connect to port %d: %d\n",
dp->index, ret);
- if (phy_is_fixed)
- of_phy_deregister_fixed_link(port_dn);
+ phylink_destroy(dp->pl);
return ret;
}
}
- phy_attached_info(slave_dev->phydev);
-
return 0;
}
@@ -1183,29 +1256,26 @@ static void dsa_slave_set_lockdep_class_one(struct net_device *dev,
int dsa_slave_suspend(struct net_device *slave_dev)
{
- struct dsa_slave_priv *p = netdev_priv(slave_dev);
+ struct dsa_port *dp = dsa_slave_to_port(slave_dev);
netif_device_detach(slave_dev);
- if (slave_dev->phydev) {
- phy_stop(slave_dev->phydev);
- p->old_pause = -1;
- p->old_link = -1;
- p->old_duplex = -1;
- phy_suspend(slave_dev->phydev);
- }
+ rtnl_lock();
+ phylink_stop(dp->pl);
+ rtnl_unlock();
return 0;
}
int dsa_slave_resume(struct net_device *slave_dev)
{
+ struct dsa_port *dp = dsa_slave_to_port(slave_dev);
+
netif_device_attach(slave_dev);
- if (slave_dev->phydev) {
- phy_resume(slave_dev->phydev);
- phy_start(slave_dev->phydev);
- }
+ rtnl_lock();
+ phylink_start(dp->pl);
+ rtnl_unlock();
return 0;
}
@@ -1270,11 +1340,6 @@ int dsa_slave_create(struct dsa_port *port)
p->dp = port;
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = cpu_dp->tag_ops->xmit;
-
- p->old_pause = -1;
- p->old_link = -1;
- p->old_duplex = -1;
-
port->slave = slave_dev;
netif_carrier_off(slave_dev);
@@ -1297,9 +1362,10 @@ int dsa_slave_create(struct dsa_port *port)
return 0;
out_phy:
- phy_disconnect(slave_dev->phydev);
- if (of_phy_is_fixed_link(port->dn))
- of_phy_deregister_fixed_link(port->dn);
+ rtnl_lock();
+ phylink_disconnect_phy(p->dp->pl);
+ rtnl_unlock();
+ phylink_destroy(p->dp->pl);
out_free:
free_percpu(p->stats64);
free_netdev(slave_dev);
@@ -1311,15 +1377,13 @@ void dsa_slave_destroy(struct net_device *slave_dev)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
struct dsa_slave_priv *p = netdev_priv(slave_dev);
- struct device_node *port_dn = dp->dn;
netif_carrier_off(slave_dev);
- if (slave_dev->phydev) {
- phy_disconnect(slave_dev->phydev);
+ rtnl_lock();
+ phylink_disconnect_phy(dp->pl);
+ rtnl_unlock();
+ phylink_destroy(dp->pl);
- if (of_phy_is_fixed_link(port_dn))
- of_phy_deregister_fixed_link(port_dn);
- }
dsa_slave_notify(slave_dev, DSA_PORT_UNREGISTER);
unregister_netdev(slave_dev);
free_percpu(p->stats64);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: dsa: bcm_sf2: Implement phylink_mac_ops
From: Florian Fainelli @ 2018-03-18 18:52 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, privat, andrew, vivien.didelot, davem,
rmk+kernel, sean.wang, Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-1-f.fainelli@gmail.com>
Make the bcm_sf2 driver implement phylink_mac_ops since it needs to
support a wide variety of network interfaces: internal & external MDIO
PHYs, fixed PHYs, MoCA with MMIO link status.
A large amount of what needs to be done already exists under
bcm_sf2_sw_adjust_link() so we are essentially breaking this down into
the necessary operation for PHYLINK to work: mac_config, mac_link_up,
mac_link_down and validate. We can now entirely get rid of most of what
fixed_link_update() provided because only the link information is actually
necessary. We still have to force DUPLEX_FULL for legacy Device Tree bindings
that did not specify that before.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 186 +++++++++++++++++++++++++++++-----------------
1 file changed, 118 insertions(+), 68 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 726d75a61795..b4d5fdcf4183 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -306,7 +306,8 @@ static int bcm_sf2_sw_mdio_write(struct mii_bus *bus, int addr, int regnum,
static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
{
- struct bcm_sf2_priv *priv = dev_id;
+ struct dsa_switch *ds = dev_id;
+ struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
priv->irq0_stat = intrl2_0_readl(priv, INTRL2_CPU_STATUS) &
~priv->irq0_mask;
@@ -317,16 +318,21 @@ static irqreturn_t bcm_sf2_switch_0_isr(int irq, void *dev_id)
static irqreturn_t bcm_sf2_switch_1_isr(int irq, void *dev_id)
{
- struct bcm_sf2_priv *priv = dev_id;
+ struct dsa_switch *ds = dev_id;
+ struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
priv->irq1_stat = intrl2_1_readl(priv, INTRL2_CPU_STATUS) &
~priv->irq1_mask;
intrl2_1_writel(priv, priv->irq1_stat, INTRL2_CPU_CLEAR);
- if (priv->irq1_stat & P_LINK_UP_IRQ(P7_IRQ_OFF))
- priv->port_sts[7].link = 1;
- if (priv->irq1_stat & P_LINK_DOWN_IRQ(P7_IRQ_OFF))
- priv->port_sts[7].link = 0;
+ if (priv->irq1_stat & P_LINK_UP_IRQ(P7_IRQ_OFF)) {
+ priv->port_sts[7].link = true;
+ dsa_port_phylink_mac_change(ds, 7, true);
+ }
+ if (priv->irq1_stat & P_LINK_DOWN_IRQ(P7_IRQ_OFF)) {
+ priv->port_sts[7].link = false;
+ dsa_port_phylink_mac_change(ds, 7, false);
+ }
return IRQ_HANDLED;
}
@@ -473,13 +479,56 @@ static u32 bcm_sf2_sw_get_phy_flags(struct dsa_switch *ds, int port)
return priv->hw_params.gphy_rev;
}
-static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
- struct phy_device *phydev)
+static void bcm_sf2_sw_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+ if (!phy_interface_mode_is_rgmii(state->interface) &&
+ state->interface != PHY_INTERFACE_MODE_MII &&
+ state->interface != PHY_INTERFACE_MODE_REVMII &&
+ state->interface != PHY_INTERFACE_MODE_GMII &&
+ state->interface != PHY_INTERFACE_MODE_INTERNAL &&
+ state->interface != PHY_INTERFACE_MODE_MOCA) {
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ dev_err(ds->dev,
+ "Unsupported interface: %d\n", state->interface);
+ return;
+ }
+
+ /* Allow all the expected bits */
+ phylink_set(mask, Autoneg);
+ phylink_set_port_modes(mask);
+ phylink_set(mask, Pause);
+ phylink_set(mask, Asym_Pause);
+
+ /* With the exclusion of MII and Reverse MII, we support Gigabit,
+ * including Half duplex
+ */
+ if (state->interface != PHY_INTERFACE_MODE_MII &&
+ state->interface != PHY_INTERFACE_MODE_REVMII) {
+ phylink_set(mask, 1000baseT_Full);
+ phylink_set(mask, 1000baseT_Half);
+ }
+
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+
+ bitmap_and(supported, supported, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static void bcm_sf2_sw_mac_config(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ const struct phylink_link_state *state)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- struct ethtool_eee *p = &priv->dev->ports[port].eee;
u32 id_mode_dis = 0, port_mode;
- const char *str = NULL;
u32 reg, offset;
if (priv->type == BCM7445_DEVICE_ID)
@@ -487,62 +536,48 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
else
offset = CORE_STS_OVERRIDE_GMIIP2_PORT(port);
- switch (phydev->interface) {
+ switch (state->interface) {
case PHY_INTERFACE_MODE_RGMII:
- str = "RGMII (no delay)";
id_mode_dis = 1;
+ /* fallthrough */
case PHY_INTERFACE_MODE_RGMII_TXID:
- if (!str)
- str = "RGMII (TX delay)";
port_mode = EXT_GPHY;
break;
case PHY_INTERFACE_MODE_MII:
- str = "MII";
port_mode = EXT_EPHY;
break;
case PHY_INTERFACE_MODE_REVMII:
- str = "Reverse MII";
port_mode = EXT_REVMII;
break;
default:
- /* All other PHYs: internal and MoCA */
- goto force_link;
- }
-
- /* If the link is down, just disable the interface to conserve power */
- if (!phydev->link) {
- reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
- reg &= ~RGMII_MODE_EN;
- reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
+ /* all other PHYs: internal and MoCA */
goto force_link;
}
- /* Clear id_mode_dis bit, and the existing port mode, but
- * make sure we enable the RGMII block for data to pass
+ /* Clear id_mode_dis bit, and the existing port mode, let
+ * RGMII_MODE_EN bet set by mac_link_{up,down}
*/
reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
reg &= ~ID_MODE_DIS;
reg &= ~(PORT_MODE_MASK << PORT_MODE_SHIFT);
reg &= ~(RX_PAUSE_EN | TX_PAUSE_EN);
- reg |= port_mode | RGMII_MODE_EN;
+ reg |= port_mode;
if (id_mode_dis)
reg |= ID_MODE_DIS;
- if (phydev->pause) {
- if (phydev->asym_pause)
+ if (state->pause & MLO_PAUSE_TXRX_MASK) {
+ if (state->pause & MLO_PAUSE_TX)
reg |= TX_PAUSE_EN;
reg |= RX_PAUSE_EN;
}
reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
- pr_info("Port %d configured for %s\n", port, str);
-
force_link:
/* Force link settings detected from the PHY */
reg = SW_OVERRIDE;
- switch (phydev->speed) {
+ switch (state->speed) {
case SPEED_1000:
reg |= SPDSTS_1000 << SPEED_SHIFT;
break;
@@ -551,33 +586,61 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
break;
}
- if (phydev->link)
+ if (state->link)
reg |= LINK_STS;
- if (phydev->duplex == DUPLEX_FULL)
+ if (state->duplex == DUPLEX_FULL)
reg |= DUPLX_MODE;
core_writel(priv, reg, offset);
-
- if (!phydev->is_pseudo_fixed_link)
- p->eee_enabled = b53_eee_init(ds, port, phydev);
}
-static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
- struct phylink_link_state *status)
+static void bcm_sf2_sw_mac_link_set(struct dsa_switch *ds, int port,
+ phy_interface_t interface, bool link)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- u32 duplex, pause, offset;
u32 reg;
- if (priv->type == BCM7445_DEVICE_ID)
- offset = CORE_STS_OVERRIDE_GMIIP_PORT(port);
+ if (!phy_interface_mode_is_rgmii(interface) &&
+ interface != PHY_INTERFACE_MODE_MII &&
+ interface != PHY_INTERFACE_MODE_REVMII)
+ return;
+
+ /* If the link is down, just disable the interface to conserve power */
+ reg = reg_readl(priv, REG_RGMII_CNTRL_P(port));
+ if (link)
+ reg |= RGMII_MODE_EN;
else
- offset = CORE_STS_OVERRIDE_GMIIP2_PORT(port);
+ reg &= ~RGMII_MODE_EN;
+ reg_writel(priv, reg, REG_RGMII_CNTRL_P(port));
+}
+
+static void bcm_sf2_sw_mac_link_down(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface)
+{
+ bcm_sf2_sw_mac_link_set(ds, port, interface, false);
+}
+
+static void bcm_sf2_sw_mac_link_up(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface,
+ struct phy_device *phydev)
+{
+ struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+ struct ethtool_eee *p = &priv->dev->ports[port].eee;
- duplex = core_readl(priv, CORE_DUPSTS);
- pause = core_readl(priv, CORE_PAUSESTS);
+ bcm_sf2_sw_mac_link_set(ds, port, interface, true);
- status->link = 0;
+ if (mode == MLO_AN_PHY && phydev)
+ p->eee_enabled = b53_eee_init(ds, port, phydev);
+}
+
+static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
+ struct phylink_link_state *status)
+{
+ struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
+
+ status->link = false;
/* MoCA port is special as we do not get link status from CORE_LNKSTS,
* which means that we need to force the link at the port override
@@ -596,26 +659,10 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
*/
if (!status->link)
netif_carrier_off(ds->ports[port].slave);
- status->duplex = 1;
+ status->duplex = DUPLEX_FULL;
} else {
- status->link = 1;
- status->duplex = !!(duplex & (1 << port));
+ status->link = true;
}
-
- reg = core_readl(priv, offset);
- reg |= SW_OVERRIDE;
- if (status->link)
- reg |= LINK_STS;
- else
- reg &= ~LINK_STS;
- core_writel(priv, reg, offset);
-
- if ((pause & (1 << port)) &&
- (pause & (1 << (port + PAUSESTS_TX_PAUSE_SHIFT))))
- status->pause |= MLO_PAUSE_TX;
-
- if (pause & (1 << port))
- status->pause |= MLO_PAUSE_TXRX_MASK;
}
static void bcm_sf2_enable_acb(struct dsa_switch *ds)
@@ -858,7 +905,10 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
.get_ethtool_stats = b53_get_ethtool_stats,
.get_sset_count = b53_get_sset_count,
.get_phy_flags = bcm_sf2_sw_get_phy_flags,
- .adjust_link = bcm_sf2_sw_adjust_link,
+ .phylink_validate = bcm_sf2_sw_validate,
+ .phylink_mac_config = bcm_sf2_sw_mac_config,
+ .phylink_mac_link_down = bcm_sf2_sw_mac_link_down,
+ .phylink_mac_link_up = bcm_sf2_sw_mac_link_up,
.fixed_link_update = bcm_sf2_sw_fixed_link_update,
.suspend = bcm_sf2_sw_suspend,
.resume = bcm_sf2_sw_resume,
@@ -1062,14 +1112,14 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
bcm_sf2_intr_disable(priv);
ret = devm_request_irq(&pdev->dev, priv->irq0, bcm_sf2_switch_0_isr, 0,
- "switch_0", priv);
+ "switch_0", ds);
if (ret < 0) {
pr_err("failed to request switch_0 IRQ\n");
goto out_mdio;
}
ret = devm_request_irq(&pdev->dev, priv->irq1, bcm_sf2_switch_1_isr, 0,
- "switch_1", priv);
+ "switch_1", ds);
if (ret < 0) {
pr_err("failed to request switch_1 IRQ\n");
goto out_mdio;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Fix missing register lock in serdes_get_stats
From: Andrew Lunn @ 2018-03-18 18:54 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, Vivien Didelot, open list
In-Reply-To: <20180318182305.24952-1-f.fainelli@gmail.com>
On Sun, Mar 18, 2018 at 11:23:05AM -0700, Florian Fainelli wrote:
> We can hit the register lock not held assertion with the following path:
>
> [ 34.170631] mv88e6085 0.1:00: Switch registers lock not held!
> [ 34.176510] CPU: 0 PID: 950 Comm: ethtool Not tainted 4.16.0-rc4 #143
> [ 34.182985] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> [ 34.189519] Backtrace:
> [ 34.192033] [<8010c4b4>] (dump_backtrace) from [<8010c788>] (show_stack+0x20/0x24)
> [ 34.199680] r6:9f5dc010 r5:00000011 r4:9f5dc010 r3:00000000
> [ 34.205434] [<8010c768>] (show_stack) from [<80679d38>] (dump_stack+0x24/0x28)
> [ 34.212719] [<80679d14>] (dump_stack) from [<804844a8>] (mv88e6xxx_read+0x70/0x7c)
> [ 34.220376] [<80484438>] (mv88e6xxx_read) from [<804870dc>] (mv88e6xxx_port_get_cmode+0x34/0x4c)
> [ 34.229257] r5:a09cd128 r4:9ee31d07
> [ 34.232880] [<804870a8>] (mv88e6xxx_port_get_cmode) from [<80487e6c>] (mv88e6352_port_has_serdes+0x24/0x64)
> [ 34.242690] r4:9f5dc010
> [ 34.245309] [<80487e48>] (mv88e6352_port_has_serdes) from [<804880b8>] (mv88e6352_serdes_get_stats+0x28/0x12c)
> [ 34.255389] r4:00000001
> [ 34.257973] [<80488090>] (mv88e6352_serdes_get_stats) from [<804811e8>] (mv88e6xxx_get_ethtool_stats+0xb0/0xc0)
> [ 34.268156] r10:00000000 r9:00000000 r8:00000000 r7:a09cd020 r6:00000001 r5:9f5dc01c
> [ 34.276052] r4:9f5dc010
> [ 34.278631] [<80481138>] (mv88e6xxx_get_ethtool_stats) from [<8064f740>] (dsa_slave_get_ethtool_stats+0xbc/0xc4)
>
> mv88e6xxx_get_ethtool_stats() calls mv88e6xxx_get_stats() which calls both
> chip->info->ops->stats_get_stats(), which holds the register lock, and
> chip->info->ops->serdes_get_stats() which does not. Have
> chip->info->ops->serdes_get_stats() be running with the register lock held to
> avoid such assertions.
>
> Fixes: 436fe17d273b ("net: dsa: mv88e6xxx: Allow the SERDES interfaces to have statistics")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Yes, i have the same patch in my backlog of patches.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: dsa: Eliminate dsa_slave_get_link()
From: Andrew Lunn @ 2018-03-18 19:12 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, privat, vivien.didelot, davem, rmk+kernel, sean.wang,
Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-2-f.fainelli@gmail.com>
On Sun, Mar 18, 2018 at 11:52:43AM -0700, Florian Fainelli wrote:
> Since we use PHYLIB to manage the per-port link indication, this will
> also be reflected correctly in the network device's carrier state, so we
> can use ethtool_op_get_link() instead.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: dsa: Plug in PHYLINK support
From: Andrew Lunn @ 2018-03-18 19:19 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, privat, vivien.didelot, davem, rmk+kernel, sean.wang,
Woojung.Huh, john, cphealy
In-Reply-To: <20180318185246.19311-4-f.fainelli@gmail.com>
> +static int dsa_slave_nway_reset(struct net_device *dev)
> +{
> + struct dsa_port *dp = dsa_slave_to_port(dev);
> +
> + return phylink_ethtool_nway_reset(dp->pl);
> +}
Hi Florian
I've seen in one of Russells trees a patch to put a phylink into
net_device. That would make a generic slave_nway_reset() possible, and
a few others as well. Maybe it makes sense to pull in that patch?
Andrew
^ permalink raw reply
* [PATCH net] net: fec: Fix unbalanced PM runtime calls
From: Florian Fainelli @ 2018-03-18 19:49 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, l.stach, Florian Fainelli, Fugang Duan, open list
When unbinding/removing the driver, we will run into the following warnings:
[ 259.655198] fec 400d1000.ethernet: 400d1000.ethernet supply phy not found, using dummy regulator
[ 259.665065] fec 400d1000.ethernet: Unbalanced pm_runtime_enable!
[ 259.672770] fec 400d1000.ethernet (unnamed net_device) (uninitialized): Invalid MAC address: 00:00:00:00:00:00
[ 259.683062] fec 400d1000.ethernet (unnamed net_device) (uninitialized): Using random MAC address: f2:3e:93:b7:29:c1
[ 259.696239] libphy: fec_enet_mii_bus: probed
Avoid these warnings by balancing the runtime PM calls during fec_drv_remove().
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/fec_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 7a7f3a42b2aa..d4604bc8eb5b 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3600,6 +3600,8 @@ fec_drv_remove(struct platform_device *pdev)
fec_enet_mii_remove(fep);
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
+ pm_runtime_put(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
of_node_put(fep->phy_node);
--
2.14.1
^ permalink raw reply related
* [bpf-next PATCH v3 00/18] bpf,sockmap: sendmsg/sendfile ULP
From: John Fastabend @ 2018-03-18 19:56 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
This series adds a BPF hook for sendmsg and senfile by using
the ULP infrastructure and sockmap. A simple pseudocode example
would be,
// load the programs
bpf_prog_load(SOCKMAP_TCP_MSG_PROG, BPF_PROG_TYPE_SK_MSG,
&obj, &msg_prog);
// lookup the sockmap
bpf_map_msg = bpf_object__find_map_by_name(obj, "my_sock_map");
// get fd for sockmap
map_fd_msg = bpf_map__fd(bpf_map_msg);
// attach program to sockmap
bpf_prog_attach(msg_prog, map_fd_msg, BPF_SK_MSG_VERDICT, 0);
// Add a socket 'fd' to sockmap at location 'i'
bpf_map_update_elem(map_fd_msg, &i, fd, BPF_ANY);
After the above snippet any socket attached to the map would run
msg_prog on sendmsg and sendfile system calls.
Three additional helpers are added bpf_msg_apply_bytes(),
bpf_msg_cork_bytes(), and bpf_msg_pull_data(). With
bpf_msg_apply_bytes BPF programs can tell the infrastructure how
many bytes the given verdict should apply to. This has two cases.
First, a BPF program applies verdict to fewer bytes than in the
current sendmsg/sendfile msg this will apply the verdict to the
first N bytes of the message then run the BPF program again with
data pointers recalculated to the N+1 byte. The second case is the
BPF program applies a verdict to more bytes than the current sendmsg
or sendfile system call. In this case the infrastructure will cache
the verdict and apply it to future sendmsg/sendfile calls until the
byte limit is reached. This avoids the overhead of running BPF
programs on large payloads.
The helper bpf_msg_cork_bytes() handles a different case where
a BPF program can not reach a verdict on a msg until it receives
more bytes AND the program doesn't want to forward the packet
until it is known to be "good". The example case being a user
(albeit a dumb one probably) sends a N byte header in 1B system
calls. The BPF program can call bpf_msg_cork_bytes with the
required byte limit to reach a verdict and then the program will
only be called again once N bytes are received.
The last helper added in this series is bpf_msg_pull_data(). It
is used to pull data in for modification or reading. Similar to
how sk_pull_data() works msg_pull_data can be used to access data
not in the initial (data_start, data_end) range. For sendpage()
calls this is needed if any data is accessed because the BPF
sendpage hook initializes the data_start and data_end pointers to
zero. We do this because sendpage data is shared with the user
and can be modified during or after the BPF verdict possibly
invalidating any verdict the BPF program decides. For sendmsg
the data is already copied by the sendmsg bpf infrastructure so
we only copy the data if the user request a data range that is
not already linearized. This happens if the user requests larger
blocks of data that are not in a single scatterlist element. The
common case seems to be accessing headers which normally are
in the first scatterlist element and already linearized.
For more examples please review the sample program. There are
examples for all the actions and helpers there.
Patches 1-8 implement the above sockmap/BPF infrastructure. The
remaining patches flush out some minimal selftests and the sample
sockmap program. The sockmap sample program is the main vehicle
for testing this infrastructure and will be moved into selftests
shortly. The final patch in this series is a simple shell script
to run a set of tests. These are the tests I run after any changes
to sockmap. The next task on the list after this series is to
push those into selftests so we can avoid manually testing.
Couple notes on future items in the pipeline,
0. move sample sockmap programs into selftests (noted above)
1. add additional support for tcp flags, most are ignored now.
2. add a Documentation/bpf/sockmap file with these details
3. support stacked ULP types to allow this and ktls to cooperate
4. Ingress flag support, redirect only supports egress here. The
other redirect helpers support ingress and egress flags.
5. add optimizations, I cut a few optimizations here in the
first iteration of the code for later study/implementation
-v3 updates
: u32 data pointers in msg_md changed to void *
: page_address NULL check and flag verification in msg_pull_data
: remove old note in commit msg that is no longer relevant
: remove enum sk_msg_action its not used anywhere
: fixup test_verifier W -> DW insn to account for data pointers
: unintentionally dropped a smap_stop_tx() call in sockmap.c
I propagated the ACKs forward because above changes were small
one/two line changes.
-v2 updates (discussion):
Dave noticed that sendpage call was previously (in v1) running
on the data directly. This allowed users to potentially modify
the data after or during the BPF program. However doing a copy
automatically even if the data is not accessed has measurable
performance impact. So we added another helper modeled after
the existing skb_pull_data() helper to allow users to selectively
pull data from the msg. This is also useful in the sendmsg case
when users need to access data outside the first scatterlist
element or across scatterlist boundaries.
While doing this I also unified the sendmsg and sendfile handlers
a bit. Originally the sendfile call was optimized for never
touching the data. I've decided for a first submission to drop
this optimization and we can add it back later. It introduced
unnecessary complexity, at least for a first posting, for a
use case I have not entirely flushed out yet. When the use
case is deployed we can add it back if needed. Then we can
review concrete performance deltas as well on real-world
use-cases/applications.
Lastly, I reorganized the patches a bit. Now all sockmap
changes are in a single patch and each helper gets its own
patch. This, at least IMO, makes it easier to review because
sockmap changes are not spread across the patch series. On
the other hand now apply_bytes, cork_bytes logic is only
activated later in the series. But that should be OK.
Thanks,
John
---
John Fastabend (18):
sock: make static tls function alloc_sg generic sock helper
sockmap: convert refcnt to an atomic refcnt
net: do_tcp_sendpages flag to avoid SKBTX_SHARED_FRAG
net: generalize sk_alloc_sg to work with scatterlist rings
bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data
bpf: sockmap, add bpf_msg_apply_bytes() helper
bpf: sockmap, add msg_cork_bytes() helper
bpf: sk_msg program helper bpf_sk_msg_pull_data
bpf: add map tests for BPF_PROG_TYPE_SK_MSG
bpf: add verifier tests for BPF_PROG_TYPE_SK_MSG
bpf: sockmap sample, add option to attach SK_MSG program
bpf: sockmap sample, add sendfile test
bpf: sockmap sample, add data verification option
bpf: sockmap, add sample option to test apply_bytes helper
bpf: sockmap sample support for bpf_msg_cork_bytes()
bpf: sockmap add SK_DROP tests
bpf: sockmap sample test for bpf_msg_pull_data
bpf: sockmap test script
include/linux/bpf.h | 1
include/linux/bpf_types.h | 1
include/linux/filter.h | 17
include/linux/socket.h | 1
include/net/sock.h | 4
include/uapi/linux/bpf.h | 25 +
kernel/bpf/sockmap.c | 733 +++++++++++++++++++-
kernel/bpf/syscall.c | 14
kernel/bpf/verifier.c | 5
net/core/filter.c | 273 +++++++
net/core/sock.c | 61 ++
net/ipv4/tcp.c | 4
net/tls/tls_sw.c | 69 --
samples/bpf/bpf_load.c | 8
samples/sockmap/sockmap_kern.c | 197 +++++
samples/sockmap/sockmap_test.sh | 450 ++++++++++++
samples/sockmap/sockmap_user.c | 301 ++++++++
tools/include/uapi/linux/bpf.h | 25 +
tools/lib/bpf/libbpf.c | 1
tools/testing/selftests/bpf/Makefile | 3
tools/testing/selftests/bpf/bpf_helpers.h | 10
tools/testing/selftests/bpf/sockmap_parse_prog.c | 15
tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c | 33 +
tools/testing/selftests/bpf/sockmap_verdict_prog.c | 7
tools/testing/selftests/bpf/test_maps.c | 55 +-
tools/testing/selftests/bpf/test_verifier.c | 54 +
26 files changed, 2236 insertions(+), 131 deletions(-)
create mode 100755 samples/sockmap/sockmap_test.sh
create mode 100644 tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c
--
Signature
^ permalink raw reply
* [bpf-next PATCH v3 01/18] sock: make static tls function alloc_sg generic sock helper
From: John Fastabend @ 2018-03-18 19:56 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
The TLS ULP module builds scatterlists from a sock using
page_frag_refill(). This is going to be useful for other ULPs
so move it into sock file for more general use.
In the process remove useless goto at end of while loop.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
include/net/sock.h | 4 +++
net/core/sock.c | 56 ++++++++++++++++++++++++++++++++++++++++++
net/tls/tls_sw.c | 69 +++++-----------------------------------------------
3 files changed, 67 insertions(+), 62 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index b962458..447150c 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2141,6 +2141,10 @@ static inline struct page_frag *sk_page_frag(struct sock *sk)
bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag);
+int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
+ int *sg_num_elem, unsigned int *sg_size,
+ int first_coalesce);
+
/*
* Default write policy as shown to user space via poll/select/SIGIO
*/
diff --git a/net/core/sock.c b/net/core/sock.c
index 27f218b..f68dff0 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2239,6 +2239,62 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
}
EXPORT_SYMBOL(sk_page_frag_refill);
+int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
+ int *sg_num_elem, unsigned int *sg_size,
+ int first_coalesce)
+{
+ struct page_frag *pfrag;
+ unsigned int size = *sg_size;
+ int num_elem = *sg_num_elem, use = 0, rc = 0;
+ struct scatterlist *sge;
+ unsigned int orig_offset;
+
+ len -= size;
+ pfrag = sk_page_frag(sk);
+
+ while (len > 0) {
+ if (!sk_page_frag_refill(sk, pfrag)) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ use = min_t(int, len, pfrag->size - pfrag->offset);
+
+ if (!sk_wmem_schedule(sk, use)) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ sk_mem_charge(sk, use);
+ size += use;
+ orig_offset = pfrag->offset;
+ pfrag->offset += use;
+
+ sge = sg + num_elem - 1;
+ if (num_elem > first_coalesce && sg_page(sg) == pfrag->page &&
+ sg->offset + sg->length == orig_offset) {
+ sg->length += use;
+ } else {
+ sge++;
+ sg_unmark_end(sge);
+ sg_set_page(sge, pfrag->page, use, orig_offset);
+ get_page(pfrag->page);
+ ++num_elem;
+ if (num_elem == MAX_SKB_FRAGS) {
+ rc = -ENOSPC;
+ break;
+ }
+ }
+
+ len -= use;
+ }
+out:
+ *sg_size = size;
+ *sg_num_elem = num_elem;
+ return rc;
+}
+EXPORT_SYMBOL(sk_alloc_sg);
+
static void __lock_sock(struct sock *sk)
__releases(&sk->sk_lock.slock)
__acquires(&sk->sk_lock.slock)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index f26376e..0fc8a24 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -87,71 +87,16 @@ static void trim_both_sgl(struct sock *sk, int target_size)
target_size);
}
-static int alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
- int *sg_num_elem, unsigned int *sg_size,
- int first_coalesce)
-{
- struct page_frag *pfrag;
- unsigned int size = *sg_size;
- int num_elem = *sg_num_elem, use = 0, rc = 0;
- struct scatterlist *sge;
- unsigned int orig_offset;
-
- len -= size;
- pfrag = sk_page_frag(sk);
-
- while (len > 0) {
- if (!sk_page_frag_refill(sk, pfrag)) {
- rc = -ENOMEM;
- goto out;
- }
-
- use = min_t(int, len, pfrag->size - pfrag->offset);
-
- if (!sk_wmem_schedule(sk, use)) {
- rc = -ENOMEM;
- goto out;
- }
-
- sk_mem_charge(sk, use);
- size += use;
- orig_offset = pfrag->offset;
- pfrag->offset += use;
-
- sge = sg + num_elem - 1;
- if (num_elem > first_coalesce && sg_page(sg) == pfrag->page &&
- sg->offset + sg->length == orig_offset) {
- sg->length += use;
- } else {
- sge++;
- sg_unmark_end(sge);
- sg_set_page(sge, pfrag->page, use, orig_offset);
- get_page(pfrag->page);
- ++num_elem;
- if (num_elem == MAX_SKB_FRAGS) {
- rc = -ENOSPC;
- break;
- }
- }
-
- len -= use;
- }
- goto out;
-
-out:
- *sg_size = size;
- *sg_num_elem = num_elem;
- return rc;
-}
-
static int alloc_encrypted_sg(struct sock *sk, int len)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
int rc = 0;
- rc = alloc_sg(sk, len, ctx->sg_encrypted_data,
- &ctx->sg_encrypted_num_elem, &ctx->sg_encrypted_size, 0);
+ rc = sk_alloc_sg(sk, len,
+ ctx->sg_encrypted_data,
+ &ctx->sg_encrypted_num_elem,
+ &ctx->sg_encrypted_size, 0);
return rc;
}
@@ -162,9 +107,9 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
int rc = 0;
- rc = alloc_sg(sk, len, ctx->sg_plaintext_data,
- &ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
- tls_ctx->pending_open_record_frags);
+ rc = sk_alloc_sg(sk, len, ctx->sg_plaintext_data,
+ &ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
+ tls_ctx->pending_open_record_frags);
return rc;
}
^ permalink raw reply related
* [bpf-next PATCH v3 02/18] sockmap: convert refcnt to an atomic refcnt
From: John Fastabend @ 2018-03-18 19:56 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
The sockmap refcnt up until now has been wrapped in the
sk_callback_lock(). So its not actually needed any locking of its
own. The counter itself tracks the lifetime of the psock object.
Sockets in a sockmap have a lifetime that is independent of the
map they are part of. This is possible because a single socket may
be in multiple maps. When this happens we can only release the
psock data associated with the socket when the refcnt reaches
zero. There are three possible delete sock reference decrement
paths first through the normal sockmap process, the user deletes
the socket from the map. Second the map is removed and all sockets
in the map are removed, delete path is similar to case 1. The third
case is an asyncronous socket event such as a closing the socket. The
last case handles removing sockets that are no longer available.
For completeness, although inc does not pose any problems in this
patch series, the inc case only happens when a psock is added to a
map.
Next we plan to add another socket prog type to handle policy and
monitoring on the TX path. When we do this however we will need to
keep a reference count open across the sendmsg/sendpage call and
holding the sk_callback_lock() here (on every send) seems less than
ideal, also it may sleep in cases where we hit memory pressure.
Instead of dealing with these issues in some clever way simply make
the reference counting a refcnt_t type and do proper atomic ops.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
kernel/bpf/sockmap.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index a927e89..051b2242 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -62,8 +62,7 @@ struct smap_psock_map_entry {
struct smap_psock {
struct rcu_head rcu;
- /* refcnt is used inside sk_callback_lock */
- u32 refcnt;
+ refcount_t refcnt;
/* datapath variables */
struct sk_buff_head rxqueue;
@@ -373,15 +372,13 @@ static void smap_destroy_psock(struct rcu_head *rcu)
static void smap_release_sock(struct smap_psock *psock, struct sock *sock)
{
- psock->refcnt--;
- if (psock->refcnt)
- return;
-
- tcp_cleanup_ulp(sock);
- smap_stop_sock(psock, sock);
- clear_bit(SMAP_TX_RUNNING, &psock->state);
- rcu_assign_sk_user_data(sock, NULL);
- call_rcu_sched(&psock->rcu, smap_destroy_psock);
+ if (refcount_dec_and_test(&psock->refcnt)) {
+ tcp_cleanup_ulp(sock);
+ smap_stop_sock(psock, sock);
+ clear_bit(SMAP_TX_RUNNING, &psock->state);
+ rcu_assign_sk_user_data(sock, NULL);
+ call_rcu_sched(&psock->rcu, smap_destroy_psock);
+ }
}
static int smap_parse_func_strparser(struct strparser *strp,
@@ -511,7 +508,7 @@ static struct smap_psock *smap_init_psock(struct sock *sock,
INIT_WORK(&psock->tx_work, smap_tx_work);
INIT_WORK(&psock->gc_work, smap_gc_work);
INIT_LIST_HEAD(&psock->maps);
- psock->refcnt = 1;
+ refcount_set(&psock->refcnt, 1);
rcu_assign_sk_user_data(sock, psock);
sock_hold(sock);
@@ -772,7 +769,7 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
err = -EBUSY;
goto out_progs;
}
- psock->refcnt++;
+ refcount_inc(&psock->refcnt);
} else {
psock = smap_init_psock(sock, stab);
if (IS_ERR(psock)) {
^ permalink raw reply related
* [bpf-next PATCH v3 03/18] net: do_tcp_sendpages flag to avoid SKBTX_SHARED_FRAG
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
When calling do_tcp_sendpages() from in kernel and we know the data
has no references from user side we can omit SKBTX_SHARED_FRAG flag.
This patch adds an internal flag, NO_SKBTX_SHARED_FRAG that can be used
to omit setting SKBTX_SHARED_FRAG.
The flag is not exposed to userspace because the sendpage call from
the splice logic masks out all bits except MSG_MORE.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
include/linux/socket.h | 1 +
net/ipv4/tcp.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 1ce1f76..60e0148 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -287,6 +287,7 @@ struct ucred {
#define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
#define MSG_BATCH 0x40000 /* sendmmsg(): more messages coming */
#define MSG_EOF MSG_FIN
+#define MSG_NO_SHARED_FRAGS 0x80000 /* sendpage() internal : page frags are not shared */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index fb350f7..f90ec24 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -994,7 +994,9 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
get_page(page);
skb_fill_page_desc(skb, i, page, offset, copy);
}
- skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
+
+ if (!(flags & MSG_NO_SHARED_FRAGS))
+ skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
skb->len += copy;
skb->data_len += copy;
^ permalink raw reply related
* [bpf-next PATCH v3 04/18] net: generalize sk_alloc_sg to work with scatterlist rings
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
The current implementation of sk_alloc_sg expects scatterlist to always
start at entry 0 and complete at entry MAX_SKB_FRAGS.
Future patches will want to support starting at arbitrary offset into
scatterlist so add an additional sg_start parameters and then default
to the current values in TLS code paths.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
include/net/sock.h | 2 +-
net/core/sock.c | 27 ++++++++++++++++-----------
net/tls/tls_sw.c | 4 ++--
3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 447150c..b7c75e0 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2142,7 +2142,7 @@ static inline struct page_frag *sk_page_frag(struct sock *sk)
bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag);
int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
- int *sg_num_elem, unsigned int *sg_size,
+ int sg_start, int *sg_curr, unsigned int *sg_size,
int first_coalesce);
/*
diff --git a/net/core/sock.c b/net/core/sock.c
index f68dff0..4f92c29 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2240,19 +2240,20 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
EXPORT_SYMBOL(sk_page_frag_refill);
int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
- int *sg_num_elem, unsigned int *sg_size,
+ int sg_start, int *sg_curr_index, unsigned int *sg_curr_size,
int first_coalesce)
{
+ int sg_curr = *sg_curr_index, use = 0, rc = 0;
+ unsigned int size = *sg_curr_size;
struct page_frag *pfrag;
- unsigned int size = *sg_size;
- int num_elem = *sg_num_elem, use = 0, rc = 0;
struct scatterlist *sge;
- unsigned int orig_offset;
len -= size;
pfrag = sk_page_frag(sk);
while (len > 0) {
+ unsigned int orig_offset;
+
if (!sk_page_frag_refill(sk, pfrag)) {
rc = -ENOMEM;
goto out;
@@ -2270,17 +2271,21 @@ int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
orig_offset = pfrag->offset;
pfrag->offset += use;
- sge = sg + num_elem - 1;
- if (num_elem > first_coalesce && sg_page(sg) == pfrag->page &&
+ sge = sg + sg_curr - 1;
+ if (sg_curr > first_coalesce && sg_page(sg) == pfrag->page &&
sg->offset + sg->length == orig_offset) {
sg->length += use;
} else {
- sge++;
+ sge = sg + sg_curr;
sg_unmark_end(sge);
sg_set_page(sge, pfrag->page, use, orig_offset);
get_page(pfrag->page);
- ++num_elem;
- if (num_elem == MAX_SKB_FRAGS) {
+ sg_curr++;
+
+ if (sg_curr == MAX_SKB_FRAGS)
+ sg_curr = 0;
+
+ if (sg_curr == sg_start) {
rc = -ENOSPC;
break;
}
@@ -2289,8 +2294,8 @@ int sk_alloc_sg(struct sock *sk, int len, struct scatterlist *sg,
len -= use;
}
out:
- *sg_size = size;
- *sg_num_elem = num_elem;
+ *sg_curr_size = size;
+ *sg_curr_index = sg_curr;
return rc;
}
EXPORT_SYMBOL(sk_alloc_sg);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0fc8a24..057a558 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -94,7 +94,7 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
int rc = 0;
rc = sk_alloc_sg(sk, len,
- ctx->sg_encrypted_data,
+ ctx->sg_encrypted_data, 0,
&ctx->sg_encrypted_num_elem,
&ctx->sg_encrypted_size, 0);
@@ -107,7 +107,7 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
int rc = 0;
- rc = sk_alloc_sg(sk, len, ctx->sg_plaintext_data,
+ rc = sk_alloc_sg(sk, len, ctx->sg_plaintext_data, 0,
&ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
tls_ctx->pending_open_record_frags);
^ permalink raw reply related
* [bpf-next PATCH v3 05/18] bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
This implements a BPF ULP layer to allow policy enforcement and
monitoring at the socket layer. In order to support this a new
program type BPF_PROG_TYPE_SK_MSG is used to run the policy at
the sendmsg/sendpage hook. To attach the policy to sockets a
sockmap is used with a new program attach type BPF_SK_MSG_VERDICT.
Similar to previous sockmap usages when a sock is added to a
sockmap, via a map update, if the map contains a BPF_SK_MSG_VERDICT
program type attached then the BPF ULP layer is created on the
socket and the attached BPF_PROG_TYPE_SK_MSG program is run for
every msg in sendmsg case and page/offset in sendpage case.
BPF_PROG_TYPE_SK_MSG Semantics/API:
BPF_PROG_TYPE_SK_MSG supports only two return codes SK_PASS and
SK_DROP. Returning SK_DROP free's the copied data in the sendmsg
case and in the sendpage case leaves the data untouched. Both cases
return -EACESS to the user. Returning SK_PASS will allow the msg to
be sent.
In the sendmsg case data is copied into kernel space buffers before
running the BPF program. The kernel space buffers are stored in a
scatterlist object where each element is a kernel memory buffer.
Some effort is made to coalesce data from the sendmsg call here.
For example a sendmsg call with many one byte iov entries will
likely be pushed into a single entry. The BPF program is run with
data pointers (start/end) pointing to the first sg element.
In the sendpage case data is not copied. We opt not to copy the
data by default here, because the BPF infrastructure does not
know what bytes will be needed nor when they will be needed. So
copying all bytes may be wasteful. Because of this the initial
start/end data pointers are (0,0). Meaning no data can be read or
written. This avoids reading data that may be modified by the
user. A new helper is added later in this series if reading and
writing the data is needed. The helper call will do a copy by
default so that the page is exclusively owned by the BPF call.
The verdict from the BPF_PROG_TYPE_SK_MSG applies to the entire msg
in the sendmsg() case and the entire page/offset in the sendpage case.
This avoids ambiguity on how to handle mixed return codes in the
sendmsg case. Again a helper is added later in the series if
a verdict needs to apply to multiple system calls and/or only
a subpart of the currently being processed message.
The helper msg_redirect_map() can be used to select the socket to
send the data on. This is used similar to existing redirect use
cases. This allows policy to redirect msgs.
Pseudo code simple example:
The basic logic to attach a program to a socket is as follows,
// load the programs
bpf_prog_load(SOCKMAP_TCP_MSG_PROG, BPF_PROG_TYPE_SK_MSG,
&obj, &msg_prog);
// lookup the sockmap
bpf_map_msg = bpf_object__find_map_by_name(obj, "my_sock_map");
// get fd for sockmap
map_fd_msg = bpf_map__fd(bpf_map_msg);
// attach program to sockmap
bpf_prog_attach(msg_prog, map_fd_msg, BPF_SK_MSG_VERDICT, 0);
Adding sockets to the map is done in the normal way,
// Add a socket 'fd' to sockmap at location 'i'
bpf_map_update_elem(map_fd_msg, &i, fd, BPF_ANY);
After the above any socket attached to "my_sock_map", in this case
'fd', will run the BPF msg verdict program (msg_prog) on every
sendmsg and sendpage system call.
For a complete example see BPF selftests or sockmap samples.
Implementation notes:
It seemed the simplest, to me at least, to use a refcnt to ensure
psock is not lost across the sendmsg copy into the sg, the bpf program
running on the data in sg_data, and the final pass to the TCP stack.
Some performance testing may show a better method to do this and avoid
the refcnt cost, but for now use the simpler method.
Another item that will come after basic support is in place is
supporting MSG_MORE flag. At the moment we call sendpages even if
the MSG_MORE flag is set. An enhancement would be to collect the
pages into a larger scatterlist and pass down the stack. Notice that
bpf_tcp_sendmsg() could support this with some additional state saved
across sendmsg calls. I built the code to support this without having
to do refactoring work. Other features TBD include ZEROCOPY and the
TCP_RECV_QUEUE/TCP_NO_QUEUE support. This will follow initial series
shortly.
Future work could improve size limits on the scatterlist rings used
here. Currently, we use MAX_SKB_FRAGS simply because this was being
used already in the TLS case. Future work could extend the kernel sk
APIs to tune this depending on workload. This is a trade-off
between memory usage and throughput performance.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/bpf.h | 1
include/linux/bpf_types.h | 1
include/linux/filter.h | 17 +
include/uapi/linux/bpf.h | 22 +
kernel/bpf/sockmap.c | 712 ++++++++++++++++++++++++++++++++++++++++++++-
kernel/bpf/syscall.c | 14 +
kernel/bpf/verifier.c | 5
net/core/filter.c | 106 +++++++
8 files changed, 857 insertions(+), 21 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 66df387..819229c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -21,6 +21,7 @@
struct perf_event;
struct bpf_prog;
struct bpf_map;
+struct sock;
/* map is generic key/value storage optionally accesible by eBPF programs */
struct bpf_map_ops {
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 19b8349..5e2e8a4 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -13,6 +13,7 @@
BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
BPF_PROG_TYPE(BPF_PROG_TYPE_SOCK_OPS, sock_ops)
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_SKB, sk_skb)
+BPF_PROG_TYPE(BPF_PROG_TYPE_SK_MSG, sk_msg)
#endif
#ifdef CONFIG_BPF_EVENTS
BPF_PROG_TYPE(BPF_PROG_TYPE_KPROBE, kprobe)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index fdb691b..109d05c 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -507,6 +507,22 @@ struct xdp_buff {
struct xdp_rxq_info *rxq;
};
+struct sk_msg_buff {
+ void *data;
+ void *data_end;
+ __u32 apply_bytes;
+ __u32 cork_bytes;
+ int sg_copybreak;
+ int sg_start;
+ int sg_curr;
+ int sg_end;
+ struct scatterlist sg_data[MAX_SKB_FRAGS];
+ bool sg_copy[MAX_SKB_FRAGS];
+ __u32 key;
+ __u32 flags;
+ struct bpf_map *map;
+};
+
/* Compute the linear packet data range [data, data_end) which
* will be accessed by various program types (cls_bpf, act_bpf,
* lwt, ...). Subsystems allowing direct data access must (!)
@@ -771,6 +787,7 @@ int xdp_do_redirect(struct net_device *dev,
void bpf_warn_invalid_xdp_action(u32 act);
struct sock *do_sk_redirect_map(struct sk_buff *skb);
+struct sock *do_msg_redirect_map(struct sk_msg_buff *md);
#ifdef CONFIG_BPF_JIT
extern int bpf_jit_enable;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1e15d17..ef52953 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -133,6 +133,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SOCK_OPS,
BPF_PROG_TYPE_SK_SKB,
BPF_PROG_TYPE_CGROUP_DEVICE,
+ BPF_PROG_TYPE_SK_MSG,
};
enum bpf_attach_type {
@@ -143,6 +144,7 @@ enum bpf_attach_type {
BPF_SK_SKB_STREAM_PARSER,
BPF_SK_SKB_STREAM_VERDICT,
BPF_CGROUP_DEVICE,
+ BPF_SK_MSG_VERDICT,
__MAX_BPF_ATTACH_TYPE
};
@@ -718,6 +720,15 @@ struct bpf_stack_build_id {
* int bpf_override_return(pt_regs, rc)
* @pt_regs: pointer to struct pt_regs
* @rc: the return value to set
+ *
+ * int bpf_msg_redirect_map(map, key, flags)
+ * Redirect msg to a sock in map using key as a lookup key for the
+ * sock in map.
+ * @map: pointer to sockmap
+ * @key: key to lookup sock in map
+ * @flags: reserved for future use
+ * Return: SK_PASS
+ *
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -779,7 +790,8 @@ struct bpf_stack_build_id {
FN(perf_prog_read_value), \
FN(getsockopt), \
FN(override_return), \
- FN(sock_ops_cb_flags_set),
+ FN(sock_ops_cb_flags_set), \
+ FN(msg_redirect_map),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -942,6 +954,14 @@ enum sk_action {
SK_PASS,
};
+/* user accessible metadata for SK_MSG packet hook, new fields must
+ * be added to the end of this structure
+ */
+struct sk_msg_md {
+ void *data;
+ void *data_end;
+};
+
#define BPF_TAG_SIZE 8
struct bpf_prog_info {
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 051b2242..69c5bcc 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -38,6 +38,7 @@
#include <linux/skbuff.h>
#include <linux/workqueue.h>
#include <linux/list.h>
+#include <linux/mm.h>
#include <net/strparser.h>
#include <net/tcp.h>
@@ -47,6 +48,7 @@
struct bpf_stab {
struct bpf_map map;
struct sock **sock_map;
+ struct bpf_prog *bpf_tx_msg;
struct bpf_prog *bpf_parse;
struct bpf_prog *bpf_verdict;
};
@@ -73,7 +75,16 @@ struct smap_psock {
int save_off;
struct sk_buff *save_skb;
+ /* datapath variables for tx_msg ULP */
+ struct sock *sk_redir;
+ int apply_bytes;
+ int cork_bytes;
+ int sg_size;
+ int eval;
+ struct sk_msg_buff *cork;
+
struct strparser strp;
+ struct bpf_prog *bpf_tx_msg;
struct bpf_prog *bpf_parse;
struct bpf_prog *bpf_verdict;
struct list_head maps;
@@ -91,6 +102,11 @@ struct smap_psock {
void (*save_write_space)(struct sock *sk);
};
+static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
+static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
+static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
+ int offset, size_t size, int flags);
+
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
{
return rcu_dereference_sk_user_data(sk);
@@ -115,27 +131,41 @@ static int bpf_tcp_init(struct sock *sk)
psock->save_close = sk->sk_prot->close;
psock->sk_proto = sk->sk_prot;
+
+ if (psock->bpf_tx_msg) {
+ tcp_bpf_proto.sendmsg = bpf_tcp_sendmsg;
+ tcp_bpf_proto.sendpage = bpf_tcp_sendpage;
+ }
+
sk->sk_prot = &tcp_bpf_proto;
rcu_read_unlock();
return 0;
}
+static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
+static int free_start_sg(struct sock *sk, struct sk_msg_buff *md);
+
static void bpf_tcp_release(struct sock *sk)
{
struct smap_psock *psock;
rcu_read_lock();
psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out;
- if (likely(psock)) {
- sk->sk_prot = psock->sk_proto;
- psock->sk_proto = NULL;
+ if (psock->cork) {
+ free_start_sg(psock->sock, psock->cork);
+ kfree(psock->cork);
+ psock->cork = NULL;
}
+
+ sk->sk_prot = psock->sk_proto;
+ psock->sk_proto = NULL;
+out:
rcu_read_unlock();
}
-static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
-
static void bpf_tcp_close(struct sock *sk, long timeout)
{
void (*close_fun)(struct sock *sk, long timeout);
@@ -174,6 +204,7 @@ enum __sk_action {
__SK_DROP = 0,
__SK_PASS,
__SK_REDIRECT,
+ __SK_NONE,
};
static struct tcp_ulp_ops bpf_tcp_ulp_ops __read_mostly = {
@@ -185,10 +216,621 @@ enum __sk_action {
.release = bpf_tcp_release,
};
+static int memcopy_from_iter(struct sock *sk,
+ struct sk_msg_buff *md,
+ struct iov_iter *from, int bytes)
+{
+ struct scatterlist *sg = md->sg_data;
+ int i = md->sg_curr, rc = -ENOSPC;
+
+ do {
+ int copy;
+ char *to;
+
+ if (md->sg_copybreak >= sg[i].length) {
+ md->sg_copybreak = 0;
+
+ if (++i == MAX_SKB_FRAGS)
+ i = 0;
+
+ if (i == md->sg_end)
+ break;
+ }
+
+ copy = sg[i].length - md->sg_copybreak;
+ to = sg_virt(&sg[i]) + md->sg_copybreak;
+ md->sg_copybreak += copy;
+
+ if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY)
+ rc = copy_from_iter_nocache(to, copy, from);
+ else
+ rc = copy_from_iter(to, copy, from);
+
+ if (rc != copy) {
+ rc = -EFAULT;
+ goto out;
+ }
+
+ bytes -= copy;
+ if (!bytes)
+ break;
+
+ md->sg_copybreak = 0;
+ if (++i == MAX_SKB_FRAGS)
+ i = 0;
+ } while (i != md->sg_end);
+out:
+ md->sg_curr = i;
+ return rc;
+}
+
+static int bpf_tcp_push(struct sock *sk, int apply_bytes,
+ struct sk_msg_buff *md,
+ int flags, bool uncharge)
+{
+ bool apply = apply_bytes;
+ struct scatterlist *sg;
+ int offset, ret = 0;
+ struct page *p;
+ size_t size;
+
+ while (1) {
+ sg = md->sg_data + md->sg_start;
+ size = (apply && apply_bytes < sg->length) ?
+ apply_bytes : sg->length;
+ offset = sg->offset;
+
+ tcp_rate_check_app_limited(sk);
+ p = sg_page(sg);
+retry:
+ ret = do_tcp_sendpages(sk, p, offset, size, flags);
+ if (ret != size) {
+ if (ret > 0) {
+ if (apply)
+ apply_bytes -= ret;
+ size -= ret;
+ offset += ret;
+ if (uncharge)
+ sk_mem_uncharge(sk, ret);
+ goto retry;
+ }
+
+ sg->length = size;
+ sg->offset = offset;
+ return ret;
+ }
+
+ if (apply)
+ apply_bytes -= ret;
+ sg->offset += ret;
+ sg->length -= ret;
+ if (uncharge)
+ sk_mem_uncharge(sk, ret);
+
+ if (!sg->length) {
+ put_page(p);
+ md->sg_start++;
+ if (md->sg_start == MAX_SKB_FRAGS)
+ md->sg_start = 0;
+ memset(sg, 0, sizeof(*sg));
+
+ if (md->sg_start == md->sg_end)
+ break;
+ }
+
+ if (apply && !apply_bytes)
+ break;
+ }
+ return 0;
+}
+
+static inline void bpf_compute_data_pointers_sg(struct sk_msg_buff *md)
+{
+ struct scatterlist *sg = md->sg_data + md->sg_start;
+
+ if (md->sg_copy[md->sg_start]) {
+ md->data = md->data_end = 0;
+ } else {
+ md->data = sg_virt(sg);
+ md->data_end = md->data + sg->length;
+ }
+}
+
+static void return_mem_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
+{
+ struct scatterlist *sg = md->sg_data;
+ int i = md->sg_start;
+
+ do {
+ int uncharge = (bytes < sg[i].length) ? bytes : sg[i].length;
+
+ sk_mem_uncharge(sk, uncharge);
+ bytes -= uncharge;
+ if (!bytes)
+ break;
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ } while (i != md->sg_end);
+}
+
+static void free_bytes_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
+{
+ struct scatterlist *sg = md->sg_data;
+ int i = md->sg_start, free;
+
+ while (bytes && sg[i].length) {
+ free = sg[i].length;
+ if (bytes < free) {
+ sg[i].length -= bytes;
+ sg[i].offset += bytes;
+ sk_mem_uncharge(sk, bytes);
+ break;
+ }
+
+ sk_mem_uncharge(sk, sg[i].length);
+ put_page(sg_page(&sg[i]));
+ bytes -= sg[i].length;
+ sg[i].length = 0;
+ sg[i].page_link = 0;
+ sg[i].offset = 0;
+ i++;
+
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ }
+}
+
+static int free_sg(struct sock *sk, int start, struct sk_msg_buff *md)
+{
+ struct scatterlist *sg = md->sg_data;
+ int i = start, free = 0;
+
+ while (sg[i].length) {
+ free += sg[i].length;
+ sk_mem_uncharge(sk, sg[i].length);
+ put_page(sg_page(&sg[i]));
+ sg[i].length = 0;
+ sg[i].page_link = 0;
+ sg[i].offset = 0;
+ i++;
+
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ }
+
+ return free;
+}
+
+static int free_start_sg(struct sock *sk, struct sk_msg_buff *md)
+{
+ int free = free_sg(sk, md->sg_start, md);
+
+ md->sg_start = md->sg_end;
+ return free;
+}
+
+static int free_curr_sg(struct sock *sk, struct sk_msg_buff *md)
+{
+ return free_sg(sk, md->sg_curr, md);
+}
+
+static int bpf_map_msg_verdict(int _rc, struct sk_msg_buff *md)
+{
+ return ((_rc == SK_PASS) ?
+ (md->map ? __SK_REDIRECT : __SK_PASS) :
+ __SK_DROP);
+}
+
+static unsigned int smap_do_tx_msg(struct sock *sk,
+ struct smap_psock *psock,
+ struct sk_msg_buff *md)
+{
+ struct bpf_prog *prog;
+ unsigned int rc, _rc;
+
+ preempt_disable();
+ rcu_read_lock();
+
+ /* If the policy was removed mid-send then default to 'accept' */
+ prog = READ_ONCE(psock->bpf_tx_msg);
+ if (unlikely(!prog)) {
+ _rc = SK_PASS;
+ goto verdict;
+ }
+
+ bpf_compute_data_pointers_sg(md);
+ rc = (*prog->bpf_func)(md, prog->insnsi);
+ psock->apply_bytes = md->apply_bytes;
+
+ /* Moving return codes from UAPI namespace into internal namespace */
+ _rc = bpf_map_msg_verdict(rc, md);
+
+ /* The psock has a refcount on the sock but not on the map and because
+ * we need to drop rcu read lock here its possible the map could be
+ * removed between here and when we need it to execute the sock
+ * redirect. So do the map lookup now for future use.
+ */
+ if (_rc == __SK_REDIRECT) {
+ if (psock->sk_redir)
+ sock_put(psock->sk_redir);
+ psock->sk_redir = do_msg_redirect_map(md);
+ if (!psock->sk_redir) {
+ _rc = __SK_DROP;
+ goto verdict;
+ }
+ sock_hold(psock->sk_redir);
+ }
+verdict:
+ rcu_read_unlock();
+ preempt_enable();
+
+ return _rc;
+}
+
+static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
+ struct sk_msg_buff *md,
+ int flags)
+{
+ struct smap_psock *psock;
+ struct scatterlist *sg;
+ int i, err, free = 0;
+
+ sg = md->sg_data;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out_rcu;
+
+ if (!refcount_inc_not_zero(&psock->refcnt))
+ goto out_rcu;
+
+ rcu_read_unlock();
+ lock_sock(sk);
+ err = bpf_tcp_push(sk, send, md, flags, false);
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ if (unlikely(err))
+ goto out;
+ return 0;
+out_rcu:
+ rcu_read_unlock();
+out:
+ i = md->sg_start;
+ while (sg[i].length) {
+ free += sg[i].length;
+ put_page(sg_page(&sg[i]));
+ sg[i].length = 0;
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ }
+ return free;
+}
+
+static inline void bpf_md_init(struct smap_psock *psock)
+{
+ if (!psock->apply_bytes) {
+ psock->eval = __SK_NONE;
+ if (psock->sk_redir) {
+ sock_put(psock->sk_redir);
+ psock->sk_redir = NULL;
+ }
+ }
+}
+
+static void apply_bytes_dec(struct smap_psock *psock, int i)
+{
+ if (psock->apply_bytes) {
+ if (psock->apply_bytes < i)
+ psock->apply_bytes = 0;
+ else
+ psock->apply_bytes -= i;
+ }
+}
+
+static int bpf_exec_tx_verdict(struct smap_psock *psock,
+ struct sk_msg_buff *m,
+ struct sock *sk,
+ int *copied, int flags)
+{
+ bool cork = false, enospc = (m->sg_start == m->sg_end);
+ struct sock *redir;
+ int err = 0;
+ int send;
+
+more_data:
+ if (psock->eval == __SK_NONE)
+ psock->eval = smap_do_tx_msg(sk, psock, m);
+
+ if (m->cork_bytes &&
+ m->cork_bytes > psock->sg_size && !enospc) {
+ psock->cork_bytes = m->cork_bytes - psock->sg_size;
+ if (!psock->cork) {
+ psock->cork = kcalloc(1,
+ sizeof(struct sk_msg_buff),
+ GFP_ATOMIC | __GFP_NOWARN);
+
+ if (!psock->cork) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+ }
+ memcpy(psock->cork, m, sizeof(*m));
+ goto out_err;
+ }
+
+ send = psock->sg_size;
+ if (psock->apply_bytes && psock->apply_bytes < send)
+ send = psock->apply_bytes;
+
+ switch (psock->eval) {
+ case __SK_PASS:
+ err = bpf_tcp_push(sk, send, m, flags, true);
+ if (unlikely(err)) {
+ *copied -= free_start_sg(sk, m);
+ break;
+ }
+
+ apply_bytes_dec(psock, send);
+ psock->sg_size -= send;
+ break;
+ case __SK_REDIRECT:
+ redir = psock->sk_redir;
+ apply_bytes_dec(psock, send);
+
+ if (psock->cork) {
+ cork = true;
+ psock->cork = NULL;
+ }
+
+ return_mem_sg(sk, send, m);
+ release_sock(sk);
+
+ err = bpf_tcp_sendmsg_do_redirect(redir, send, m, flags);
+ lock_sock(sk);
+
+ if (cork) {
+ free_start_sg(sk, m);
+ kfree(m);
+ m = NULL;
+ }
+ if (unlikely(err))
+ *copied -= err;
+ else
+ psock->sg_size -= send;
+ break;
+ case __SK_DROP:
+ default:
+ free_bytes_sg(sk, send, m);
+ apply_bytes_dec(psock, send);
+ *copied -= send;
+ psock->sg_size -= send;
+ err = -EACCES;
+ break;
+ }
+
+ if (likely(!err)) {
+ bpf_md_init(psock);
+ if (m &&
+ m->sg_data[m->sg_start].page_link &&
+ m->sg_data[m->sg_start].length)
+ goto more_data;
+ }
+
+out_err:
+ return err;
+}
+
+static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
+{
+ int flags = msg->msg_flags | MSG_NO_SHARED_FRAGS;
+ struct sk_msg_buff md = {0};
+ unsigned int sg_copy = 0;
+ struct smap_psock *psock;
+ int copied = 0, err = 0;
+ struct scatterlist *sg;
+ long timeo;
+
+ /* Its possible a sock event or user removed the psock _but_ the ops
+ * have not been reprogrammed yet so we get here. In this case fallback
+ * to tcp_sendmsg. Note this only works because we _only_ ever allow
+ * a single ULP there is no hierarchy here.
+ */
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ return tcp_sendmsg(sk, msg, size);
+ }
+
+ /* Increment the psock refcnt to ensure its not released while sending a
+ * message. Required because sk lookup and bpf programs are used in
+ * separate rcu critical sections. Its OK if we lose the map entry
+ * but we can't lose the sock reference.
+ */
+ if (!refcount_inc_not_zero(&psock->refcnt)) {
+ rcu_read_unlock();
+ return tcp_sendmsg(sk, msg, size);
+ }
+
+ sg = md.sg_data;
+ sg_init_table(sg, MAX_SKB_FRAGS);
+ rcu_read_unlock();
+
+ lock_sock(sk);
+ timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
+
+ while (msg_data_left(msg)) {
+ struct sk_msg_buff *m;
+ bool enospc = false;
+ int copy;
+
+ if (sk->sk_err) {
+ err = sk->sk_err;
+ goto out_err;
+ }
+
+ copy = msg_data_left(msg);
+ if (!sk_stream_memory_free(sk))
+ goto wait_for_sndbuf;
+
+ m = psock->cork_bytes ? psock->cork : &md;
+ m->sg_curr = m->sg_copybreak ? m->sg_curr : m->sg_end;
+ err = sk_alloc_sg(sk, copy, m->sg_data,
+ m->sg_start, &m->sg_end, &sg_copy,
+ m->sg_end - 1);
+ if (err) {
+ if (err != -ENOSPC)
+ goto wait_for_memory;
+ enospc = true;
+ copy = sg_copy;
+ }
+
+ err = memcopy_from_iter(sk, m, &msg->msg_iter, copy);
+ if (err < 0) {
+ free_curr_sg(sk, m);
+ goto out_err;
+ }
+
+ psock->sg_size += copy;
+ copied += copy;
+ sg_copy = 0;
+
+ /* When bytes are being corked skip running BPF program and
+ * applying verdict unless there is no more buffer space. In
+ * the ENOSPC case simply run BPF prorgram with currently
+ * accumulated data. We don't have much choice at this point
+ * we could try extending the page frags or chaining complex
+ * frags but even in these cases _eventually_ we will hit an
+ * OOM scenario. More complex recovery schemes may be
+ * implemented in the future, but BPF programs must handle
+ * the case where apply_cork requests are not honored. The
+ * canonical method to verify this is to check data length.
+ */
+ if (psock->cork_bytes) {
+ if (copy > psock->cork_bytes)
+ psock->cork_bytes = 0;
+ else
+ psock->cork_bytes -= copy;
+
+ if (psock->cork_bytes && !enospc)
+ goto out_cork;
+
+ /* All cork bytes accounted for re-run filter */
+ psock->eval = __SK_NONE;
+ psock->cork_bytes = 0;
+ }
+
+ err = bpf_exec_tx_verdict(psock, m, sk, &copied, flags);
+ if (unlikely(err < 0))
+ goto out_err;
+ continue;
+wait_for_sndbuf:
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+wait_for_memory:
+ err = sk_stream_wait_memory(sk, &timeo);
+ if (err)
+ goto out_err;
+ }
+out_err:
+ if (err < 0)
+ err = sk_stream_error(sk, msg->msg_flags, err);
+out_cork:
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return copied ? copied : err;
+}
+
+static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
+ int offset, size_t size, int flags)
+{
+ struct sk_msg_buff md = {0}, *m = NULL;
+ int err = 0, copied = 0;
+ struct smap_psock *psock;
+ struct scatterlist *sg;
+ bool enospc = false;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto accept;
+
+ if (!refcount_inc_not_zero(&psock->refcnt))
+ goto accept;
+ rcu_read_unlock();
+
+ lock_sock(sk);
+
+ if (psock->cork_bytes)
+ m = psock->cork;
+ else
+ m = &md;
+
+ /* Catch case where ring is full and sendpage is stalled. */
+ if (unlikely(m->sg_end == m->sg_start &&
+ m->sg_data[m->sg_end].length))
+ goto out_err;
+
+ psock->sg_size += size;
+ sg = &m->sg_data[m->sg_end];
+ sg_set_page(sg, page, size, offset);
+ get_page(page);
+ m->sg_copy[m->sg_end] = true;
+ sk_mem_charge(sk, size);
+ m->sg_end++;
+ copied = size;
+
+ if (m->sg_end == MAX_SKB_FRAGS)
+ m->sg_end = 0;
+
+ if (m->sg_end == m->sg_start)
+ enospc = true;
+
+ if (psock->cork_bytes) {
+ if (size > psock->cork_bytes)
+ psock->cork_bytes = 0;
+ else
+ psock->cork_bytes -= size;
+
+ if (psock->cork_bytes && !enospc)
+ goto out_err;
+
+ /* All cork bytes accounted for re-run filter */
+ psock->eval = __SK_NONE;
+ psock->cork_bytes = 0;
+ }
+
+ err = bpf_exec_tx_verdict(psock, m, sk, &copied, flags);
+out_err:
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return copied ? copied : err;
+accept:
+ rcu_read_unlock();
+ return tcp_sendpage(sk, page, offset, size, flags);
+}
+
+static void bpf_tcp_msg_add(struct smap_psock *psock,
+ struct sock *sk,
+ struct bpf_prog *tx_msg)
+{
+ struct bpf_prog *orig_tx_msg;
+
+ orig_tx_msg = xchg(&psock->bpf_tx_msg, tx_msg);
+ if (orig_tx_msg)
+ bpf_prog_put(orig_tx_msg);
+}
+
static int bpf_tcp_ulp_register(void)
{
tcp_bpf_proto = tcp_prot;
tcp_bpf_proto.close = bpf_tcp_close;
+ /* Once BPF TX ULP is registered it is never unregistered. It
+ * will be in the ULP list for the lifetime of the system. Doing
+ * duplicate registers is not a problem.
+ */
return tcp_register_ulp(&bpf_tcp_ulp_ops);
}
@@ -412,7 +1054,6 @@ static int smap_parse_func_strparser(struct strparser *strp,
return rc;
}
-
static int smap_read_sock_done(struct strparser *strp, int err)
{
return err;
@@ -482,12 +1123,22 @@ static void smap_gc_work(struct work_struct *w)
bpf_prog_put(psock->bpf_parse);
if (psock->bpf_verdict)
bpf_prog_put(psock->bpf_verdict);
+ if (psock->bpf_tx_msg)
+ bpf_prog_put(psock->bpf_tx_msg);
+
+ if (psock->cork) {
+ free_start_sg(psock->sock, psock->cork);
+ kfree(psock->cork);
+ }
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
list_del(&e->list);
kfree(e);
}
+ if (psock->sk_redir)
+ sock_put(psock->sk_redir);
+
sock_put(psock->sock);
kfree(psock);
}
@@ -503,6 +1154,7 @@ static struct smap_psock *smap_init_psock(struct sock *sock,
if (!psock)
return ERR_PTR(-ENOMEM);
+ psock->eval = __SK_NONE;
psock->sock = sock;
skb_queue_head_init(&psock->rxqueue);
INIT_WORK(&psock->tx_work, smap_tx_work);
@@ -711,10 +1363,11 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
{
struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
struct smap_psock_map_entry *e = NULL;
- struct bpf_prog *verdict, *parse;
+ struct bpf_prog *verdict, *parse, *tx_msg;
struct sock *osock, *sock;
struct smap_psock *psock;
u32 i = *(u32 *)key;
+ bool new = false;
int err;
if (unlikely(flags > BPF_EXIST))
@@ -737,6 +1390,7 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
*/
verdict = READ_ONCE(stab->bpf_verdict);
parse = READ_ONCE(stab->bpf_parse);
+ tx_msg = READ_ONCE(stab->bpf_tx_msg);
if (parse && verdict) {
/* bpf prog refcnt may be zero if a concurrent attach operation
@@ -755,6 +1409,17 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
}
}
+ if (tx_msg) {
+ tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg);
+ if (IS_ERR(tx_msg)) {
+ if (verdict)
+ bpf_prog_put(verdict);
+ if (parse)
+ bpf_prog_put(parse);
+ return PTR_ERR(tx_msg);
+ }
+ }
+
write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
@@ -769,7 +1434,14 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
err = -EBUSY;
goto out_progs;
}
- refcount_inc(&psock->refcnt);
+ if (READ_ONCE(psock->bpf_tx_msg) && tx_msg) {
+ err = -EBUSY;
+ goto out_progs;
+ }
+ if (!refcount_inc_not_zero(&psock->refcnt)) {
+ err = -EAGAIN;
+ goto out_progs;
+ }
} else {
psock = smap_init_psock(sock, stab);
if (IS_ERR(psock)) {
@@ -777,11 +1449,8 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
goto out_progs;
}
- err = tcp_set_ulp_id(sock, TCP_ULP_BPF);
- if (err)
- goto out_progs;
-
set_bit(SMAP_TX_RUNNING, &psock->state);
+ new = true;
}
e = kzalloc(sizeof(*e), GFP_ATOMIC | __GFP_NOWARN);
@@ -794,6 +1463,14 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
/* 3. At this point we have a reference to a valid psock that is
* running. Attach any BPF programs needed.
*/
+ if (tx_msg)
+ bpf_tcp_msg_add(psock, sock, tx_msg);
+ if (new) {
+ err = tcp_set_ulp_id(sock, TCP_ULP_BPF);
+ if (err)
+ goto out_free;
+ }
+
if (parse && verdict && !psock->strp_enabled) {
err = smap_init_sock(psock, sock);
if (err)
@@ -815,8 +1492,6 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
struct smap_psock *opsock = smap_psock_sk(osock);
write_lock_bh(&osock->sk_callback_lock);
- if (osock != sock && parse)
- smap_stop_sock(opsock, osock);
smap_list_remove(opsock, &stab->sock_map[i]);
smap_release_sock(opsock, osock);
write_unlock_bh(&osock->sk_callback_lock);
@@ -829,6 +1504,8 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
bpf_prog_put(verdict);
if (parse)
bpf_prog_put(parse);
+ if (tx_msg)
+ bpf_prog_put(tx_msg);
write_unlock_bh(&sock->sk_callback_lock);
kfree(e);
return err;
@@ -843,6 +1520,9 @@ int sock_map_prog(struct bpf_map *map, struct bpf_prog *prog, u32 type)
return -EINVAL;
switch (type) {
+ case BPF_SK_MSG_VERDICT:
+ orig = xchg(&stab->bpf_tx_msg, prog);
+ break;
case BPF_SK_SKB_STREAM_PARSER:
orig = xchg(&stab->bpf_parse, prog);
break;
@@ -904,6 +1584,10 @@ static void sock_map_release(struct bpf_map *map, struct file *map_file)
orig = xchg(&stab->bpf_verdict, NULL);
if (orig)
bpf_prog_put(orig);
+
+ orig = xchg(&stab->bpf_tx_msg, NULL);
+ if (orig)
+ bpf_prog_put(orig);
}
const struct bpf_map_ops sock_map_ops = {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e24aa32..3aeb4ea 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1315,7 +1315,8 @@ static int bpf_obj_get(const union bpf_attr *attr)
#define BPF_PROG_ATTACH_LAST_FIELD attach_flags
-static int sockmap_get_from_fd(const union bpf_attr *attr, bool attach)
+static int sockmap_get_from_fd(const union bpf_attr *attr,
+ int type, bool attach)
{
struct bpf_prog *prog = NULL;
int ufd = attr->target_fd;
@@ -1329,8 +1330,7 @@ static int sockmap_get_from_fd(const union bpf_attr *attr, bool attach)
return PTR_ERR(map);
if (attach) {
- prog = bpf_prog_get_type(attr->attach_bpf_fd,
- BPF_PROG_TYPE_SK_SKB);
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, type);
if (IS_ERR(prog)) {
fdput(f);
return PTR_ERR(prog);
@@ -1382,9 +1382,11 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_CGROUP_DEVICE:
ptype = BPF_PROG_TYPE_CGROUP_DEVICE;
break;
+ case BPF_SK_MSG_VERDICT:
+ return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_MSG, true);
case BPF_SK_SKB_STREAM_PARSER:
case BPF_SK_SKB_STREAM_VERDICT:
- return sockmap_get_from_fd(attr, true);
+ return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, true);
default:
return -EINVAL;
}
@@ -1437,9 +1439,11 @@ static int bpf_prog_detach(const union bpf_attr *attr)
case BPF_CGROUP_DEVICE:
ptype = BPF_PROG_TYPE_CGROUP_DEVICE;
break;
+ case BPF_SK_MSG_VERDICT:
+ return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_MSG, false);
case BPF_SK_SKB_STREAM_PARSER:
case BPF_SK_SKB_STREAM_VERDICT:
- return sockmap_get_from_fd(attr, false);
+ return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, false);
default:
return -EINVAL;
}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index eb79a34..e9f7c20 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1248,6 +1248,7 @@ static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
case BPF_PROG_TYPE_XDP:
case BPF_PROG_TYPE_LWT_XMIT:
case BPF_PROG_TYPE_SK_SKB:
+ case BPF_PROG_TYPE_SK_MSG:
if (meta)
return meta->pkt_access;
@@ -2071,7 +2072,8 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
case BPF_MAP_TYPE_SOCKMAP:
if (func_id != BPF_FUNC_sk_redirect_map &&
func_id != BPF_FUNC_sock_map_update &&
- func_id != BPF_FUNC_map_delete_elem)
+ func_id != BPF_FUNC_map_delete_elem &&
+ func_id != BPF_FUNC_msg_redirect_map)
goto error;
break;
default:
@@ -2109,6 +2111,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
goto error;
break;
case BPF_FUNC_sk_redirect_map:
+ case BPF_FUNC_msg_redirect_map:
if (map->map_type != BPF_MAP_TYPE_SOCKMAP)
goto error;
break;
diff --git a/net/core/filter.c b/net/core/filter.c
index 33edfa8..2b6c475 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1890,6 +1890,44 @@ struct sock *do_sk_redirect_map(struct sk_buff *skb)
.arg4_type = ARG_ANYTHING,
};
+BPF_CALL_4(bpf_msg_redirect_map, struct sk_msg_buff *, msg,
+ struct bpf_map *, map, u32, key, u64, flags)
+{
+ /* If user passes invalid input drop the packet. */
+ if (unlikely(flags))
+ return SK_DROP;
+
+ msg->key = key;
+ msg->flags = flags;
+ msg->map = map;
+
+ return SK_PASS;
+}
+
+struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
+{
+ struct sock *sk = NULL;
+
+ if (msg->map) {
+ sk = __sock_map_lookup_elem(msg->map, msg->key);
+
+ msg->key = 0;
+ msg->map = NULL;
+ }
+
+ return sk;
+}
+
+static const struct bpf_func_proto bpf_msg_redirect_map_proto = {
+ .func = bpf_msg_redirect_map,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_CONST_MAP_PTR,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_ANYTHING,
+};
+
BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
{
return task_get_classid(skb);
@@ -3591,6 +3629,16 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
}
}
+static const struct bpf_func_proto *sk_msg_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_msg_redirect_map:
+ return &bpf_msg_redirect_map_proto;
+ default:
+ return bpf_base_func_proto(func_id);
+ }
+}
+
static const struct bpf_func_proto *sk_skb_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
@@ -3980,6 +4028,32 @@ static bool sk_skb_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, info);
}
+static bool sk_msg_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ struct bpf_insn_access_aux *info)
+{
+ if (type == BPF_WRITE)
+ return false;
+
+ switch (off) {
+ case offsetof(struct sk_msg_md, data):
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case offsetof(struct sk_msg_md, data_end):
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ }
+
+ if (off < 0 || off >= sizeof(struct sk_msg_md))
+ return false;
+ if (off % size != 0)
+ return false;
+ if (size != sizeof(__u64))
+ return false;
+
+ return true;
+}
+
static u32 bpf_convert_ctx_access(enum bpf_access_type type,
const struct bpf_insn *si,
struct bpf_insn *insn_buf,
@@ -4778,6 +4852,29 @@ static u32 sk_skb_convert_ctx_access(enum bpf_access_type type,
return insn - insn_buf;
}
+static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
+ const struct bpf_insn *si,
+ struct bpf_insn *insn_buf,
+ struct bpf_prog *prog, u32 *target_size)
+{
+ struct bpf_insn *insn = insn_buf;
+
+ switch (si->off) {
+ case offsetof(struct sk_msg_md, data):
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_msg_buff, data),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, data));
+ break;
+ case offsetof(struct sk_msg_md, data_end):
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_msg_buff, data_end),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, data_end));
+ break;
+ }
+
+ return insn - insn_buf;
+}
+
const struct bpf_verifier_ops sk_filter_verifier_ops = {
.get_func_proto = sk_filter_func_proto,
.is_valid_access = sk_filter_is_valid_access,
@@ -4868,6 +4965,15 @@ static u32 sk_skb_convert_ctx_access(enum bpf_access_type type,
const struct bpf_prog_ops sk_skb_prog_ops = {
};
+const struct bpf_verifier_ops sk_msg_verifier_ops = {
+ .get_func_proto = sk_msg_func_proto,
+ .is_valid_access = sk_msg_is_valid_access,
+ .convert_ctx_access = sk_msg_convert_ctx_access,
+};
+
+const struct bpf_prog_ops sk_msg_prog_ops = {
+};
+
int sk_detach_filter(struct sock *sk)
{
int ret = -ENOENT;
^ permalink raw reply related
* [bpf-next PATCH v3 06/18] bpf: sockmap, add bpf_msg_apply_bytes() helper
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
A single sendmsg or sendfile system call can contain multiple logical
messages that a BPF program may want to read and apply a verdict. But,
without an apply_bytes helper any verdict on the data applies to all
bytes in the sendmsg/sendfile. Alternatively, a BPF program may only
care to read the first N bytes of a msg. If the payload is large say
MB or even GB setting up and calling the BPF program repeatedly for
all bytes, even though the verdict is already known, creates
unnecessary overhead.
To allow BPF programs to control how many bytes a given verdict
applies to we implement a bpf_msg_apply_bytes() helper. When called
from within a BPF program this sets a counter, internal to the
BPF infrastructure, that applies the last verdict to the next N
bytes. If the N is smaller than the current data being processed
from a sendmsg/sendfile call, the first N bytes will be sent and
the BPF program will be re-run with start_data pointing to the N+1
byte. If N is larger than the current data being processed the
BPF verdict will be applied to multiple sendmsg/sendfile calls
until N bytes are consumed.
Note1 if a socket closes with apply_bytes counter non-zero this
is not a problem because data is not being buffered for N bytes
and is sent as its received.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 3 ++-
net/core/filter.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ef52953..a557a2a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -791,7 +791,8 @@ struct bpf_stack_build_id {
FN(getsockopt), \
FN(override_return), \
FN(sock_ops_cb_flags_set), \
- FN(msg_redirect_map),
+ FN(msg_redirect_map), \
+ FN(msg_apply_bytes),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 2b6c475..17d6775 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1928,6 +1928,20 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
.arg4_type = ARG_ANYTHING,
};
+BPF_CALL_2(bpf_msg_apply_bytes, struct sk_msg_buff *, msg, u32, bytes)
+{
+ msg->apply_bytes = bytes;
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_msg_apply_bytes_proto = {
+ .func = bpf_msg_apply_bytes,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+};
+
BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
{
return task_get_classid(skb);
@@ -3634,6 +3648,8 @@ static const struct bpf_func_proto *sk_msg_func_proto(enum bpf_func_id func_id)
switch (func_id) {
case BPF_FUNC_msg_redirect_map:
return &bpf_msg_redirect_map_proto;
+ case BPF_FUNC_msg_apply_bytes:
+ return &bpf_msg_apply_bytes_proto;
default:
return bpf_base_func_proto(func_id);
}
^ permalink raw reply related
* [bpf-next PATCH v3 07/18] bpf: sockmap, add msg_cork_bytes() helper
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
In the case where we need a specific number of bytes before a
verdict can be assigned, even if the data spans multiple sendmsg
or sendfile calls. The BPF program may use msg_cork_bytes().
The extreme case is a user can call sendmsg repeatedly with
1-byte msg segments. Obviously, this is bad for performance but
is still valid. If the BPF program needs N bytes to validate
a header it can use msg_cork_bytes to specify N bytes and the
BPF program will not be called again until N bytes have been
accumulated. The infrastructure will attempt to coalesce data
if possible so in many cases (most my use cases at least) the
data will be in a single scatterlist element with data pointers
pointing to start/end of the element. However, this is dependent
on available memory so is not guaranteed. So BPF programs must
validate data pointer ranges, but this is the case anyways to
convince the verifier the accesses are valid.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 3 ++-
net/core/filter.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a557a2a..1765cfb 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -792,7 +792,8 @@ struct bpf_stack_build_id {
FN(override_return), \
FN(sock_ops_cb_flags_set), \
FN(msg_redirect_map), \
- FN(msg_apply_bytes),
+ FN(msg_apply_bytes), \
+ FN(msg_cork_bytes),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 17d6775..0c9daf6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1942,6 +1942,20 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
.arg2_type = ARG_ANYTHING,
};
+BPF_CALL_2(bpf_msg_cork_bytes, struct sk_msg_buff *, msg, u32, bytes)
+{
+ msg->cork_bytes = bytes;
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_msg_cork_bytes_proto = {
+ .func = bpf_msg_cork_bytes,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+};
+
BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
{
return task_get_classid(skb);
@@ -3650,6 +3664,8 @@ static const struct bpf_func_proto *sk_msg_func_proto(enum bpf_func_id func_id)
return &bpf_msg_redirect_map_proto;
case BPF_FUNC_msg_apply_bytes:
return &bpf_msg_apply_bytes_proto;
+ case BPF_FUNC_msg_cork_bytes:
+ return &bpf_msg_cork_bytes_proto;
default:
return bpf_base_func_proto(func_id);
}
^ permalink raw reply related
* [bpf-next PATCH v3 08/18] bpf: sk_msg program helper bpf_sk_msg_pull_data
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
Currently, if a bpf sk msg program is run the program
can only parse data that the (start,end) pointers already
consumed. For sendmsg hooks this is likely the first
scatterlist element. For sendpage this will be the range
(0,0) because the data is shared with userspace and by
default we want to avoid allowing userspace to modify
data while (or after) BPF verdict is being decided.
To support pulling in additional bytes for parsing use
a new helper bpf_sk_msg_pull(start, end, flags) which
works similar to cls tc logic. This helper will attempt
to point the data start pointer at 'start' bytes offest
into msg and data end pointer at 'end' bytes offset into
message.
After basic sanity checks to ensure 'start' <= 'end' and
'end' <= msg_length there are a few cases we need to
handle.
First the sendmsg hook has already copied the data from
userspace and has exclusive access to it. Therefor, it
is not necessesary to copy the data. However, it may
be required. After finding the scatterlist element with
'start' offset byte in it there are two cases. One the
range (start,end) is entirely contained in the sg element
and is already linear. All that is needed is to update the
data pointers, no allocate/copy is needed. The other case
is (start, end) crosses sg element boundaries. In this
case we allocate a block of size 'end - start' and copy
the data to linearize it.
Next sendpage hook has not copied any data in initial
state so that data pointers are (0,0). In this case we
handle it similar to the above sendmsg case except the
allocation/copy must always happen. Then when sending
the data we have possibly three memory regions that
need to be sent, (0, start - 1), (start, end), and
(end + 1, msg_length). This is required to ensure any
writes by the BPF program are correctly transmitted.
Lastly this operation will invalidate any previous
data checks so BPF programs will have to revalidate
pointers after making this BPF call.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 3 +
net/core/filter.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1765cfb..18b7c51 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -793,7 +793,8 @@ struct bpf_stack_build_id {
FN(sock_ops_cb_flags_set), \
FN(msg_redirect_map), \
FN(msg_apply_bytes), \
- FN(msg_cork_bytes),
+ FN(msg_cork_bytes), \
+ FN(msg_pull_data),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 0c9daf6..c86f03f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1956,6 +1956,136 @@ struct sock *do_msg_redirect_map(struct sk_msg_buff *msg)
.arg2_type = ARG_ANYTHING,
};
+BPF_CALL_4(bpf_msg_pull_data,
+ struct sk_msg_buff *, msg, u32, start, u32, end, u64, flags)
+{
+ unsigned int len = 0, offset = 0, copy = 0;
+ struct scatterlist *sg = msg->sg_data;
+ int first_sg, last_sg, i, shift;
+ unsigned char *p, *to, *from;
+ int bytes = end - start;
+ struct page *page;
+
+ if (unlikely(flags || end <= start))
+ return -EINVAL;
+
+ /* First find the starting scatterlist element */
+ i = msg->sg_start;
+ do {
+ len = sg[i].length;
+ offset += len;
+ if (start < offset + len)
+ break;
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ } while (i != msg->sg_end);
+
+ if (unlikely(start >= offset + len))
+ return -EINVAL;
+
+ if (!msg->sg_copy[i] && bytes <= len)
+ goto out;
+
+ first_sg = i;
+
+ /* At this point we need to linearize multiple scatterlist
+ * elements or a single shared page. Either way we need to
+ * copy into a linear buffer exclusively owned by BPF. Then
+ * place the buffer in the scatterlist and fixup the original
+ * entries by removing the entries now in the linear buffer
+ * and shifting the remaining entries. For now we do not try
+ * to copy partial entries to avoid complexity of running out
+ * of sg_entry slots. The downside is reading a single byte
+ * will copy the entire sg entry.
+ */
+ do {
+ copy += sg[i].length;
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ if (bytes < copy)
+ break;
+ } while (i != msg->sg_end);
+ last_sg = i;
+
+ if (unlikely(copy < end - start))
+ return -EINVAL;
+
+ page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC, get_order(copy));
+ if (unlikely(!page))
+ return -ENOMEM;
+ p = page_address(page);
+ offset = 0;
+
+ i = first_sg;
+ do {
+ from = sg_virt(&sg[i]);
+ len = sg[i].length;
+ to = p + offset;
+
+ memcpy(to, from, len);
+ offset += len;
+ sg[i].length = 0;
+ put_page(sg_page(&sg[i]));
+
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ } while (i != last_sg);
+
+ sg[first_sg].length = copy;
+ sg_set_page(&sg[first_sg], page, copy, 0);
+
+ /* To repair sg ring we need to shift entries. If we only
+ * had a single entry though we can just replace it and
+ * be done. Otherwise walk the ring and shift the entries.
+ */
+ shift = last_sg - first_sg - 1;
+ if (!shift)
+ goto out;
+
+ i = first_sg + 1;
+ do {
+ int move_from;
+
+ if (i + shift >= MAX_SKB_FRAGS)
+ move_from = i + shift - MAX_SKB_FRAGS;
+ else
+ move_from = i + shift;
+
+ if (move_from == msg->sg_end)
+ break;
+
+ sg[i] = sg[move_from];
+ sg[move_from].length = 0;
+ sg[move_from].page_link = 0;
+ sg[move_from].offset = 0;
+
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ } while (1);
+ msg->sg_end -= shift;
+ if (msg->sg_end < 0)
+ msg->sg_end += MAX_SKB_FRAGS;
+out:
+ msg->data = sg_virt(&sg[i]) + start - offset;
+ msg->data_end = msg->data + bytes;
+
+ return 0;
+}
+
+static const struct bpf_func_proto bpf_msg_pull_data_proto = {
+ .func = bpf_msg_pull_data,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_ANYTHING,
+};
+
BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
{
return task_get_classid(skb);
@@ -2897,7 +3027,8 @@ bool bpf_helper_changes_pkt_data(void *func)
func == bpf_l3_csum_replace ||
func == bpf_l4_csum_replace ||
func == bpf_xdp_adjust_head ||
- func == bpf_xdp_adjust_meta)
+ func == bpf_xdp_adjust_meta ||
+ func == bpf_msg_pull_data)
return true;
return false;
@@ -3666,6 +3797,8 @@ static const struct bpf_func_proto *sk_msg_func_proto(enum bpf_func_id func_id)
return &bpf_msg_apply_bytes_proto;
case BPF_FUNC_msg_cork_bytes:
return &bpf_msg_cork_bytes_proto;
+ case BPF_FUNC_msg_pull_data:
+ return &bpf_msg_pull_data_proto;
default:
return bpf_base_func_proto(func_id);
}
^ permalink raw reply related
* [bpf-next PATCH v3 09/18] bpf: add map tests for BPF_PROG_TYPE_SK_MSG
From: John Fastabend @ 2018-03-18 19:57 UTC (permalink / raw)
To: davejwatson, davem, daniel, ast; +Cc: netdev
In-Reply-To: <20180318195501.14466.25366.stgit@john-Precision-Tower-5810>
Add map tests to attach BPF_PROG_TYPE_SK_MSG types to a sockmap.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
---
tools/include/uapi/linux/bpf.h | 10 ++++
tools/testing/selftests/bpf/Makefile | 3 +
tools/testing/selftests/bpf/bpf_helpers.h | 2 +
tools/testing/selftests/bpf/sockmap_parse_prog.c | 15 +++++
tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c | 33 ++++++++++++
tools/testing/selftests/bpf/sockmap_verdict_prog.c | 7 +++
tools/testing/selftests/bpf/test_maps.c | 55 +++++++++++++++++++-
7 files changed, 118 insertions(+), 7 deletions(-)
create mode 100644 tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1944d0a..13d9c59 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -133,6 +133,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SOCK_OPS,
BPF_PROG_TYPE_SK_SKB,
BPF_PROG_TYPE_CGROUP_DEVICE,
+ BPF_PROG_TYPE_SK_MSG,
};
enum bpf_attach_type {
@@ -143,6 +144,7 @@ enum bpf_attach_type {
BPF_SK_SKB_STREAM_PARSER,
BPF_SK_SKB_STREAM_VERDICT,
BPF_CGROUP_DEVICE,
+ BPF_SK_MSG_VERDICT,
__MAX_BPF_ATTACH_TYPE
};
@@ -941,6 +943,14 @@ enum sk_action {
SK_PASS,
};
+/* user accessible metadata for SK_MSG packet hook, new fields must
+ * be added to the end of this structure
+ */
+struct sk_msg_md {
+ void *data;
+ void *data_end;
+};
+
#define BPF_TAG_SIZE 8
struct bpf_prog_info {
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index b0d29fd..f35fb02 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -29,7 +29,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \
sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \
- sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o
+ sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
+ sockmap_tcp_msg_prog.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index dde2c11..1558fe8 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -123,6 +123,8 @@ static int (*bpf_skb_under_cgroup)(void *ctx, void *map, int index) =
(void *) BPF_FUNC_skb_under_cgroup;
static int (*bpf_skb_change_head)(void *, int len, int flags) =
(void *) BPF_FUNC_skb_change_head;
+static int (*bpf_skb_pull_data)(void *, int len) =
+ (void *) BPF_FUNC_skb_pull_data;
/* Scan the ARCH passed in from ARCH env variable (see Makefile) */
#if defined(__TARGET_ARCH_x86)
diff --git a/tools/testing/selftests/bpf/sockmap_parse_prog.c b/tools/testing/selftests/bpf/sockmap_parse_prog.c
index a1dec2b..0f92858 100644
--- a/tools/testing/selftests/bpf/sockmap_parse_prog.c
+++ b/tools/testing/selftests/bpf/sockmap_parse_prog.c
@@ -20,14 +20,25 @@ int bpf_prog1(struct __sk_buff *skb)
__u32 lport = skb->local_port;
__u32 rport = skb->remote_port;
__u8 *d = data;
+ __u32 len = (__u32) data_end - (__u32) data;
+ int err;
- if (data + 10 > data_end)
- return skb->len;
+ if (data + 10 > data_end) {
+ err = bpf_skb_pull_data(skb, 10);
+ if (err)
+ return SK_DROP;
+
+ data_end = (void *)(long)skb->data_end;
+ data = (void *)(long)skb->data;
+ if (data + 10 > data_end)
+ return SK_DROP;
+ }
/* This write/read is a bit pointless but tests the verifier and
* strparser handler for read/write pkt data and access into sk
* fields.
*/
+ d = data;
d[7] = 1;
return skb->len;
}
diff --git a/tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c b/tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c
new file mode 100644
index 0000000..12a7b5c
--- /dev/null
+++ b/tools/testing/selftests/bpf/sockmap_tcp_msg_prog.c
@@ -0,0 +1,33 @@
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+#include "bpf_util.h"
+#include "bpf_endian.h"
+
+int _version SEC("version") = 1;
+
+#define bpf_printk(fmt, ...) \
+({ \
+ char ____fmt[] = fmt; \
+ bpf_trace_printk(____fmt, sizeof(____fmt), \
+ ##__VA_ARGS__); \
+})
+
+SEC("sk_msg1")
+int bpf_prog1(struct sk_msg_md *msg)
+{
+ void *data_end = (void *)(long) msg->data_end;
+ void *data = (void *)(long) msg->data;
+
+ char *d;
+
+ if (data + 8 > data_end)
+ return SK_DROP;
+
+ bpf_printk("data length %i\n", (__u64)msg->data_end - (__u64)msg->data);
+ d = (char *)data;
+ bpf_printk("hello sendmsg hook %i %i\n", d[0], d[1]);
+
+ return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/sockmap_verdict_prog.c b/tools/testing/selftests/bpf/sockmap_verdict_prog.c
index d7bea97..2ce7634 100644
--- a/tools/testing/selftests/bpf/sockmap_verdict_prog.c
+++ b/tools/testing/selftests/bpf/sockmap_verdict_prog.c
@@ -26,6 +26,13 @@ struct bpf_map_def SEC("maps") sock_map_tx = {
.max_entries = 20,
};
+struct bpf_map_def SEC("maps") sock_map_msg = {
+ .type = BPF_MAP_TYPE_SOCKMAP,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 20,
+};
+
struct bpf_map_def SEC("maps") sock_map_break = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(int),
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 1238733..6c25334 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -464,15 +464,17 @@ static void test_devmap(int task, void *data)
#include <linux/err.h>
#define SOCKMAP_PARSE_PROG "./sockmap_parse_prog.o"
#define SOCKMAP_VERDICT_PROG "./sockmap_verdict_prog.o"
+#define SOCKMAP_TCP_MSG_PROG "./sockmap_tcp_msg_prog.o"
static void test_sockmap(int tasks, void *data)
{
- int one = 1, map_fd_rx = 0, map_fd_tx = 0, map_fd_break, s, sc, rc;
- struct bpf_map *bpf_map_rx, *bpf_map_tx, *bpf_map_break;
+ struct bpf_map *bpf_map_rx, *bpf_map_tx, *bpf_map_msg, *bpf_map_break;
+ int map_fd_msg = 0, map_fd_rx = 0, map_fd_tx = 0, map_fd_break;
int ports[] = {50200, 50201, 50202, 50204};
int err, i, fd, udp, sfd[6] = {0xdeadbeef};
u8 buf[20] = {0x0, 0x5, 0x3, 0x2, 0x1, 0x0};
- int parse_prog, verdict_prog;
+ int parse_prog, verdict_prog, msg_prog;
struct sockaddr_in addr;
+ int one = 1, s, sc, rc;
struct bpf_object *obj;
struct timeval to;
__u32 key, value;
@@ -584,6 +586,12 @@ static void test_sockmap(int tasks, void *data)
goto out_sockmap;
}
+ err = bpf_prog_attach(-1, fd, BPF_SK_MSG_VERDICT, 0);
+ if (!err) {
+ printf("Failed invalid msg verdict prog attach\n");
+ goto out_sockmap;
+ }
+
err = bpf_prog_attach(-1, fd, __MAX_BPF_ATTACH_TYPE, 0);
if (!err) {
printf("Failed unknown prog attach\n");
@@ -602,6 +610,12 @@ static void test_sockmap(int tasks, void *data)
goto out_sockmap;
}
+ err = bpf_prog_detach(fd, BPF_SK_MSG_VERDICT);
+ if (err) {
+ printf("Failed empty msg verdict prog detach\n");
+ goto out_sockmap;
+ }
+
err = bpf_prog_detach(fd, __MAX_BPF_ATTACH_TYPE);
if (!err) {
printf("Detach invalid prog successful\n");
@@ -616,6 +630,13 @@ static void test_sockmap(int tasks, void *data)
goto out_sockmap;
}
+ err = bpf_prog_load(SOCKMAP_TCP_MSG_PROG,
+ BPF_PROG_TYPE_SK_MSG, &obj, &msg_prog);
+ if (err) {
+ printf("Failed to load SK_SKB msg prog\n");
+ goto out_sockmap;
+ }
+
err = bpf_prog_load(SOCKMAP_VERDICT_PROG,
BPF_PROG_TYPE_SK_SKB, &obj, &verdict_prog);
if (err) {
@@ -631,7 +652,7 @@ static void test_sockmap(int tasks, void *data)
map_fd_rx = bpf_map__fd(bpf_map_rx);
if (map_fd_rx < 0) {
- printf("Failed to get map fd\n");
+ printf("Failed to get map rx fd\n");
goto out_sockmap;
}
@@ -647,6 +668,18 @@ static void test_sockmap(int tasks, void *data)
goto out_sockmap;
}
+ bpf_map_msg = bpf_object__find_map_by_name(obj, "sock_map_msg");
+ if (IS_ERR(bpf_map_msg)) {
+ printf("Failed to load map msg from msg_verdict prog\n");
+ goto out_sockmap;
+ }
+
+ map_fd_msg = bpf_map__fd(bpf_map_msg);
+ if (map_fd_msg < 0) {
+ printf("Failed to get map msg fd\n");
+ goto out_sockmap;
+ }
+
bpf_map_break = bpf_object__find_map_by_name(obj, "sock_map_break");
if (IS_ERR(bpf_map_break)) {
printf("Failed to load map tx from verdict prog\n");
@@ -680,6 +713,12 @@ static void test_sockmap(int tasks, void *data)
goto out_sockmap;
}
+ err = bpf_prog_attach(msg_prog, map_fd_msg, BPF_SK_MSG_VERDICT, 0);
+ if (err) {
+ printf("Failed msg verdict bpf prog attach\n");
+ goto out_sockmap;
+ }
+
err = bpf_prog_attach(verdict_prog, map_fd_rx,
__MAX_BPF_ATTACH_TYPE, 0);
if (!err) {
@@ -719,6 +758,14 @@ static void test_sockmap(int tasks, void *data)
}
}
+ /* Put sfd[2] (sending fd below) into msg map to test sendmsg bpf */
+ i = 0;
+ err = bpf_map_update_elem(map_fd_msg, &i, &sfd[2], BPF_ANY);
+ if (err) {
+ printf("Failed map_fd_msg update sockmap %i\n", err);
+ goto out_sockmap;
+ }
+
/* Test map send/recv */
for (i = 0; i < 2; i++) {
buf[0] = i;
^ 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