* [PATCH net-next 5/5] net: gemini: Indicate that we can handle jumboframes
From: Linus Walleij @ 2018-06-30 16:18 UTC (permalink / raw)
To: netdev, David S . Miller, Michał Mirosław
Cc: Janos Laube, Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
Florian Fainelli, Linus Walleij
In-Reply-To: <20180630161806.24312-1-linus.walleij@linaro.org>
The hardware supposedly handles frames up to 10236 bytes and
implements .ndo_change_mtu() so accept 10236 minus the ethernet
header for a VLAN tagged frame on the netdevices.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/cortina/gemini.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 79324bbfd768..ae475393e4ac 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -2473,6 +2473,11 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
netdev->hw_features = GMAC_OFFLOAD_FEATURES;
netdev->features |= GMAC_OFFLOAD_FEATURES | NETIF_F_GRO;
+ /* We can handle jumbo frames up to 10236 bytes so, let's accept
+ * payloads of 10236 bytes minus VLAN and ethernet header
+ */
+ netdev->min_mtu = 256;
+ netdev->max_mtu = 10236 - VLAN_ETH_HLEN;
port->freeq_refill = 0;
netif_napi_add(netdev, &port->napi, gmac_napi_poll,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: gemini: Move main init to port
From: Linus Walleij @ 2018-06-30 16:18 UTC (permalink / raw)
To: netdev, David S . Miller, Michał Mirosław
Cc: Janos Laube, Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
Florian Fainelli, Linus Walleij
In-Reply-To: <20180630161806.24312-1-linus.walleij@linaro.org>
The initialization sequence for the ethernet, setting up
interrupt routing and such things, need to be done after
both the ports are clocked and reset. Before this the
config will not "take". Move the initialization to the
port probe function and keep track of init status in
the state.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/cortina/gemini.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 8d192fcd51c8..79324bbfd768 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -146,6 +146,7 @@ struct gemini_ethernet {
void __iomem *base;
struct gemini_ethernet_port *port0;
struct gemini_ethernet_port *port1;
+ bool initialized;
spinlock_t irq_lock; /* Locks IRQ-related registers */
unsigned int freeq_order;
@@ -2301,6 +2302,14 @@ static void gemini_port_remove(struct gemini_ethernet_port *port)
static void gemini_ethernet_init(struct gemini_ethernet *geth)
{
+ /* Only do this once both ports are online */
+ if (geth->initialized)
+ return;
+ if (geth->port0 && geth->port1)
+ geth->initialized = true;
+ else
+ return;
+
writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_0_REG);
writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_1_REG);
writel(0, geth->base + GLOBAL_INTERRUPT_ENABLE_2_REG);
@@ -2447,6 +2456,10 @@ static int gemini_ethernet_port_probe(struct platform_device *pdev)
geth->port0 = port;
else
geth->port1 = port;
+
+ /* This will just be done once both ports are up and reset */
+ gemini_ethernet_init(geth);
+
platform_set_drvdata(pdev, port);
/* Set up and register the netdev */
@@ -2564,7 +2577,6 @@ static int gemini_ethernet_probe(struct platform_device *pdev)
spin_lock_init(&geth->irq_lock);
spin_lock_init(&geth->freeq_lock);
- gemini_ethernet_init(geth);
/* The children will use this */
platform_set_drvdata(pdev, geth);
@@ -2577,8 +2589,8 @@ static int gemini_ethernet_remove(struct platform_device *pdev)
{
struct gemini_ethernet *geth = platform_get_drvdata(pdev);
- gemini_ethernet_init(geth);
geth_cleanup_freeq(geth);
+ geth->initialized = false;
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: gemini: Allow multiple ports to instantiate
From: Linus Walleij @ 2018-06-30 16:18 UTC (permalink / raw)
To: netdev, David S . Miller, Michał Mirosław
Cc: Janos Laube, Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
Florian Fainelli, Linus Walleij
In-Reply-To: <20180630161806.24312-1-linus.walleij@linaro.org>
The code was not tested with two ports actually in use at
the same time. (I blame this on lack of actual hardware using
that feature.) Now after locating a system using both ports,
add necessary fix to make both ports come up.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/cortina/gemini.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index b49ed8964026..8d192fcd51c8 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -1787,7 +1787,10 @@ static int gmac_open(struct net_device *netdev)
phy_start(netdev->phydev);
err = geth_resize_freeq(port);
- if (err) {
+ /* It's fine if it's just busy, the other port has set up
+ * the freeq in that case.
+ */
+ if (err && (err != -EBUSY)) {
netdev_err(netdev, "could not resize freeq\n");
goto err_stop_phy;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/5] net: gemini: Improve connection prints
From: Linus Walleij @ 2018-06-30 16:18 UTC (permalink / raw)
To: netdev, David S . Miller, Michał Mirosław
Cc: Janos Laube, Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
Florian Fainelli, Linus Walleij
In-Reply-To: <20180630161806.24312-1-linus.walleij@linaro.org>
Instead of just specify that a PHY is connected at some
speed, also specify which one. This is helpful with several
PHYs on the system.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/cortina/gemini.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 8fc31723f700..b49ed8964026 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -300,23 +300,26 @@ static void gmac_speed_set(struct net_device *netdev)
status.bits.speed = GMAC_SPEED_1000;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
status.bits.mii_rmii = GMAC_PHY_RGMII_1000;
- netdev_info(netdev, "connect to RGMII @ 1Gbit\n");
+ netdev_info(netdev, "connect %s to RGMII @ 1Gbit\n",
+ phydev_name(phydev));
break;
case 100:
status.bits.speed = GMAC_SPEED_100;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
status.bits.mii_rmii = GMAC_PHY_RGMII_100_10;
- netdev_info(netdev, "connect to RGMII @ 100 Mbit\n");
+ netdev_info(netdev, "connect %s to RGMII @ 100 Mbit\n",
+ phydev_name(phydev));
break;
case 10:
status.bits.speed = GMAC_SPEED_10;
if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
status.bits.mii_rmii = GMAC_PHY_RGMII_100_10;
- netdev_info(netdev, "connect to RGMII @ 10 Mbit\n");
+ netdev_info(netdev, "connect %s to RGMII @ 10 Mbit\n",
+ phydev_name(phydev));
break;
default:
- netdev_warn(netdev, "Not supported PHY speed (%d)\n",
- phydev->speed);
+ netdev_warn(netdev, "Unsupported PHY speed (%d) on %s\n",
+ phydev->speed, phydev_name(phydev));
}
if (phydev->duplex == DUPLEX_FULL) {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/5] net: gemini: Look up L3 maxlen from table
From: Linus Walleij @ 2018-06-30 16:18 UTC (permalink / raw)
To: netdev, David S . Miller, Michał Mirosław
Cc: Janos Laube, Paulius Zaleckas, linux-arm-kernel, Hans Ulli Kroll,
Florian Fainelli, Linus Walleij
The code to calculate the hardware register enumerator
for the maximum L3 length isn't entirely simple to read.
Use the existing defines and rewrite the function into a
table look-up.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/ethernet/cortina/gemini.c | 61 ++++++++++++++++++++-------
1 file changed, 46 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
index 6d7404f66f84..8fc31723f700 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -401,26 +401,57 @@ static int gmac_setup_phy(struct net_device *netdev)
return 0;
}
-static int gmac_pick_rx_max_len(int max_l3_len)
-{
- /* index = CONFIG_MAXLEN_XXX values */
- static const int max_len[8] = {
- 1536, 1518, 1522, 1542,
- 9212, 10236, 1518, 1518
- };
- int i, n = 5;
+/* The maximum frame length is not logically enumerated in the
+ * hardware, so we do a table lookup to find the applicable max
+ * frame length.
+ */
+struct gmac_max_framelen {
+ unsigned int max_l3_len;
+ u8 val;
+};
- max_l3_len += ETH_HLEN + VLAN_HLEN;
+static const struct gmac_max_framelen gmac_maxlens[] = {
+ {
+ .max_l3_len = 1518,
+ .val = CONFIG0_MAXLEN_1518,
+ },
+ {
+ .max_l3_len = 1522,
+ .val = CONFIG0_MAXLEN_1522,
+ },
+ {
+ .max_l3_len = 1536,
+ .val = CONFIG0_MAXLEN_1536,
+ },
+ {
+ .max_l3_len = 1542,
+ .val = CONFIG0_MAXLEN_1542,
+ },
+ {
+ .max_l3_len = 9212,
+ .val = CONFIG0_MAXLEN_9k,
+ },
+ {
+ .max_l3_len = 10236,
+ .val = CONFIG0_MAXLEN_10k,
+ },
+};
+
+static int gmac_pick_rx_max_len(unsigned int max_l3_len)
+{
+ const struct gmac_max_framelen *maxlen;
+ int maxtot;
+ int i;
- if (max_l3_len > max_len[n])
- return -1;
+ maxtot = max_l3_len + ETH_HLEN + VLAN_HLEN;
- for (i = 0; i < 5; i++) {
- if (max_len[i] >= max_l3_len && max_len[i] < max_len[n])
- n = i;
+ for (i = 0; i < ARRAY_SIZE(gmac_maxlens); i++) {
+ maxlen = &gmac_maxlens[i];
+ if (maxtot <= maxlen->max_l3_len)
+ return maxlen->val;
}
- return n;
+ return -1;
}
static int gmac_init(struct net_device *netdev)
--
2.17.1
^ permalink raw reply related
* Re: [net-next 01/12] net/mlx5e: Add UDP GSO support
From: Boris Pismenny @ 2018-06-30 16:06 UTC (permalink / raw)
To: Willem de Bruijn, Saeed Mahameed
Cc: David Miller, Network Development, ogerlitz, yossiku,
Alexander Duyck
In-Reply-To: <CAF=yD-LHN6GS7aGBm3enQDJNrvrUp6aGGMcr7bHOR0PNrLqKJA@mail.gmail.com>
On 06/30/18 01:19, Willem de Bruijn wrote:
> On Fri, Jun 29, 2018 at 2:24 AM Saeed Mahameed <saeedm@mellanox.com> wrote:
>> From: Boris Pismenny <borisp@mellanox.com>
>>
>> This patch enables UDP GSO support. We enable this by using two WQEs
>> the first is a UDP LSO WQE for all segments with equal length, and the
>> second is for the last segment in case it has different length.
>> Due to HW limitation, before sending, we must adjust the packet length fields.
>>
>> We measure performance between two Intel(R) Xeon(R) CPU E5-2643 v2 @3.50GHz
>> machines connected back-to-back with Connectx4-Lx (40Gbps) NICs.
>> We compare single stream UDP, UDP GSO and UDP GSO with offload.
>> Performance:
>> | MSS (bytes) | Throughput (Gbps) | CPU utilization (%)
>> UDP GSO offload | 1472 | 35.6 | 8%
>> UDP GSO | 1472 | 25.5 | 17%
>> UDP | 1472 | 10.2 | 17%
>> UDP GSO offload | 1024 | 35.6 | 8%
>> UDP GSO | 1024 | 19.2 | 17%
>> UDP | 1024 | 5.7 | 17%
>> UDP GSO offload | 512 | 33.8 | 16%
>> UDP GSO | 512 | 10.4 | 17%
>> UDP | 512 | 3.5 | 17%
> Very nice results :)
Thanks. We expect to have 100Gbps results by netdev0x12.
>> +static void mlx5e_udp_gso_prepare_last_skb(struct sk_buff *skb,
>> + struct sk_buff *nskb,
>> + int remaining)
>> +{
>> + int bytes_needed = remaining, remaining_headlen, remaining_page_offset;
>> + int headlen = skb_transport_offset(skb) + sizeof(struct udphdr);
>> + int payload_len = remaining + sizeof(struct udphdr);
>> + int k = 0, i, j;
>> +
>> + skb_copy_bits(skb, 0, nskb->data, headlen);
>> + nskb->dev = skb->dev;
>> + skb_reset_mac_header(nskb);
>> + skb_set_network_header(nskb, skb_network_offset(skb));
>> + skb_set_transport_header(nskb, skb_transport_offset(skb));
>> + skb_set_tail_pointer(nskb, headlen);
>> +
>> + /* How many frags do we need? */
>> + for (i = skb_shinfo(skb)->nr_frags - 1; i >= 0; i--) {
>> + bytes_needed -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
>> + k++;
>> + if (bytes_needed <= 0)
>> + break;
>> + }
>> +
>> + /* Fill the first frag and split it if necessary */
>> + j = skb_shinfo(skb)->nr_frags - k;
>> + remaining_page_offset = -bytes_needed;
>> + skb_fill_page_desc(nskb, 0,
>> + skb_shinfo(skb)->frags[j].page.p,
>> + skb_shinfo(skb)->frags[j].page_offset + remaining_page_offset,
>> + skb_shinfo(skb)->frags[j].size - remaining_page_offset);
>> +
>> + skb_frag_ref(skb, j);
>> +
>> + /* Fill the rest of the frags */
>> + for (i = 1; i < k; i++) {
>> + j = skb_shinfo(skb)->nr_frags - k + i;
>> +
>> + skb_fill_page_desc(nskb, i,
>> + skb_shinfo(skb)->frags[j].page.p,
>> + skb_shinfo(skb)->frags[j].page_offset,
>> + skb_shinfo(skb)->frags[j].size);
>> + skb_frag_ref(skb, j);
>> + }
>> + skb_shinfo(nskb)->nr_frags = k;
>> +
>> + remaining_headlen = remaining - skb->data_len;
>> +
>> + /* headlen contains remaining data? */
>> + if (remaining_headlen > 0)
>> + skb_copy_bits(skb, skb->len - remaining, nskb->data + headlen,
>> + remaining_headlen);
>> + nskb->len = remaining + headlen;
>> + nskb->data_len = payload_len - sizeof(struct udphdr) +
>> + max_t(int, 0, remaining_headlen);
>> + nskb->protocol = skb->protocol;
>> + if (nskb->protocol == htons(ETH_P_IP)) {
>> + ip_hdr(nskb)->id = htons(ntohs(ip_hdr(nskb)->id) +
>> + skb_shinfo(skb)->gso_segs);
>> + ip_hdr(nskb)->tot_len =
>> + htons(payload_len + sizeof(struct iphdr));
>> + } else {
>> + ipv6_hdr(nskb)->payload_len = htons(payload_len);
>> + }
>> + udp_hdr(nskb)->len = htons(payload_len);
>> + skb_shinfo(nskb)->gso_size = 0;
>> + nskb->ip_summed = skb->ip_summed;
>> + nskb->csum_start = skb->csum_start;
>> + nskb->csum_offset = skb->csum_offset;
>> + nskb->queue_mapping = skb->queue_mapping;
>> +}
>> +
>> +/* might send skbs and update wqe and pi */
>> +struct sk_buff *mlx5e_udp_gso_handle_tx_skb(struct net_device *netdev,
>> + struct mlx5e_txqsq *sq,
>> + struct sk_buff *skb,
>> + struct mlx5e_tx_wqe **wqe,
>> + u16 *pi)
>> +{
>> + int payload_len = skb_shinfo(skb)->gso_size + sizeof(struct udphdr);
>> + int headlen = skb_transport_offset(skb) + sizeof(struct udphdr);
>> + int remaining = (skb->len - headlen) % skb_shinfo(skb)->gso_size;
>> + struct sk_buff *nskb;
>> +
>> + if (skb->protocol == htons(ETH_P_IP))
>> + ip_hdr(skb)->tot_len = htons(payload_len + sizeof(struct iphdr));
>> + else
>> + ipv6_hdr(skb)->payload_len = htons(payload_len);
>> + udp_hdr(skb)->len = htons(payload_len);
>> + if (!remaining)
>> + return skb;
>> +
>> + nskb = alloc_skb(max_t(int, headlen, headlen + remaining - skb->data_len), GFP_ATOMIC);
>> + if (unlikely(!nskb)) {
>> + sq->stats->dropped++;
>> + return NULL;
>> + }
>> +
>> + mlx5e_udp_gso_prepare_last_skb(skb, nskb, remaining);
>> +
>> + skb_shinfo(skb)->gso_segs--;
>> + pskb_trim(skb, skb->len - remaining);
>> + mlx5e_sq_xmit(sq, skb, *wqe, *pi);
>> + mlx5e_sq_fetch_wqe(sq, wqe, pi);
>> + return nskb;
>> +}
> The device driver seems to be implementing the packet split here
> similar to NETIF_F_GSO_PARTIAL. When advertising the right flag, the
> stack should be able to do that for you and pass two packets to the
> driver.
We've totally missed that!
Thank you. I'll fix and resubmit.
^ permalink raw reply
* [PATCH net] net: fix use-after-free in GRO with ESP
From: Sabrina Dubroca @ 2018-06-30 15:38 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, Stefano Brivio, Steffen Klassert
Since the addition of GRO for ESP, gro_receive can consume the skb and
return -EINPROGRESS. In that case, the lower layer GRO handler cannot
touch the skb anymore.
Commit 5f114163f2f5 ("net: Add a skb_gro_flush_final helper.") converted
some of the gro_receive handlers that can lead to ESP's gro_receive so
that they wouldn't access the skb when -EINPROGRESS is returned, but
missed other spots, mainly in tunneling protocols.
This patch finishes the conversion to using skb_gro_flush_final(), and
adds a new helper, skb_gro_flush_final_remcsum(), used in VXLAN and
GUE.
Fixes: 5f114163f2f5 ("net: Add a skb_gro_flush_final helper.")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/net/geneve.c | 2 +-
drivers/net/vxlan.c | 4 +---
include/linux/netdevice.h | 20 ++++++++++++++++++++
net/8021q/vlan.c | 2 +-
net/ipv4/fou.c | 4 +---
net/ipv4/gre_offload.c | 2 +-
net/ipv4/udp_offload.c | 2 +-
7 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 750eaa53bf0c..ada33c2d9ac2 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -476,7 +476,7 @@ static struct sk_buff **geneve_gro_receive(struct sock *sk,
out_unlock:
rcu_read_unlock();
out:
- NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_flush_final(skb, pp, flush);
return pp;
}
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index aee0e60471f1..f6bb1d54d4bd 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -623,9 +623,7 @@ static struct sk_buff **vxlan_gro_receive(struct sock *sk,
flush = 0;
out:
- skb_gro_remcsum_cleanup(skb, &grc);
- skb->remcsum_offload = 0;
- NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
return pp;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ec9850c7936..3d0cc0b5cec2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2789,11 +2789,31 @@ static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff **pp,
if (PTR_ERR(pp) != -EINPROGRESS)
NAPI_GRO_CB(skb)->flush |= flush;
}
+static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
+ struct sk_buff **pp,
+ int flush,
+ struct gro_remcsum *grc)
+{
+ if (PTR_ERR(pp) != -EINPROGRESS) {
+ NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_remcsum_cleanup(skb, grc);
+ skb->remcsum_offload = 0;
+ }
+}
#else
static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff **pp, int flush)
{
NAPI_GRO_CB(skb)->flush |= flush;
}
+static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
+ struct sk_buff **pp,
+ int flush,
+ struct gro_remcsum *grc)
+{
+ NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_remcsum_cleanup(skb, grc);
+ skb->remcsum_offload = 0;
+}
#endif
static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 73a65789271b..8ccee3d01822 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -693,7 +693,7 @@ static struct sk_buff **vlan_gro_receive(struct sk_buff **head,
out_unlock:
rcu_read_unlock();
out:
- NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_flush_final(skb, pp, flush);
return pp;
}
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 1540db65241a..c9ec1603666b 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -448,9 +448,7 @@ static struct sk_buff **gue_gro_receive(struct sock *sk,
out_unlock:
rcu_read_unlock();
out:
- NAPI_GRO_CB(skb)->flush |= flush;
- skb_gro_remcsum_cleanup(skb, &grc);
- skb->remcsum_offload = 0;
+ skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
return pp;
}
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 1859c473b21a..6a7d980105f6 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -223,7 +223,7 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
out_unlock:
rcu_read_unlock();
out:
- NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_flush_final(skb, pp, flush);
return pp;
}
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 92dc9e5a7ff3..69c54540d5b4 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -394,7 +394,7 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
out_unlock:
rcu_read_unlock();
out:
- NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_flush_final(skb, pp, flush);
return pp;
}
EXPORT_SYMBOL(udp_gro_receive);
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 2/2] cdc_ncm: Admit multicast traffic
From: Miguel Rodríguez Pérez @ 2018-06-30 14:22 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-usb, netdev
In-Reply-To: <87r2kozcx0.fsf@miraculix.mork.no>
On 30/06/18 14:17, Bjørn Mork wrote:
> Miguel Rodríguez Pérez <miguel@det.uvigo.gal> writes:
>
>> +static void cdc_ncm_update_filter(struct usbnet *dev)
>> +{
>> + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
>> + u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
>> + struct net_device *net = dev->net;
>
> Why not change usbnet_cdc_update_filter to get the interface number from
> dev->intf instead? Then you can export it and reuse it here instead of
> creating a copy. And it will also work for any other usbnet minidriver.
Well, I just wanted to limit my changes to the minimum, as I lack
experience with kernel development. Where should I declare
usbnet_cdc_update then? Would linux/usb/cdc.h the right place? Is it
enough to do EXPORT_GPL_SYMBOL(usbnet_cdc_update_filter) in cdc_ether.c
for it to be loaded automatically from cdc_ncm.c?
>
> I.e. change this:
>
> static void usbnet_cdc_update_filter(struct usbnet *dev)
> {
> struct cdc_state *info = (void *) &dev->data;
> struct usb_interface *intf = info->control;
>
> into:
>
> static void usbnet_cdc_update_filter(struct usbnet *dev)
> {
> struct usb_interface *intf = dev->intf;
>
>
> or simply use dev->intf to deref directly into an interface number, like
> you do in your version. The local 'intf' variable doesn't do much
> good here.
>
Ok, done.
>
>
>> +
>> + u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED
>> + | USB_CDC_PACKET_TYPE_BROADCAST;
>> +
>> + /* filtering on the device is an optional feature and not worth
>> + * the hassle so we just roughly care about snooping and if any
>> + * multicast is requested, we take every multicast
>> + */
>> + if (net->flags & IFF_PROMISC)
>> + cdc_filter |= USB_CDC_PACKET_TYPE_PROMISCUOUS;
>> + if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI))
>> + cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST;
>> +
>> + usbnet_write_cmd(dev,
>
> This is a nice change. It should be probably done in
> usbnet_cdc_update_filter in any case. Unless there is some reason it
> doesn't alreay use usbnet_write_cmd?
I don't know of any, so I'll keep it changed.
>
>> + USB_CDC_SET_ETHERNET_PACKET_FILTER,
>> + USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE,
>> + cdc_filter,
>> + iface_no,
>> + NULL,
>> + 0);
>> +}
>> +
>> static const struct ethtool_ops cdc_ncm_ethtool_ops = {
>> .get_link = usbnet_get_link,
>> .nway_reset = usbnet_nway_reset,
>> @@ -1652,6 +1679,7 @@ static const struct driver_info cdc_ncm_info = {
>> .status = cdc_ncm_status,
>> .rx_fixup = cdc_ncm_rx_fixup,
>> .tx_fixup = cdc_ncm_tx_fixup,
>> + .set_rx_mode = cdc_ncm_update_filter,
>> };
>>
>> /* Same as cdc_ncm_info, but with FLAG_WWAN */
>
>
> As the comment indicates: There are more 'struct driver_info' variants
> here. Please add .set_rx_mode to all of them, unless you have a reason
> not to?
I simply lack access to the hardware to test and I didn't want to break
anybody else's hardware. In any case, the change seems quite safe, so I
modified them all too.
I will for your comments before sending the updated version of the patches.
>
>
> Bjørn
>
--
Miguel Rodríguez Pérez
Laboratorio de Redes
EE Telecomunicación – Universidade de Vigo
^ permalink raw reply
* Re: [PATCH 1/2] cdc_ncm: Hook into usbnet_change_mtu respecting usbnet,driver_info
From: Miguel Rodríguez Pérez @ 2018-06-30 14:22 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-usb, netdev
In-Reply-To: <87vaa0zdnx.fsf@miraculix.mork.no>
On 30/06/18 14:01, Bjørn Mork wrote:
> Miguel Rodríguez Pérez <miguel@det.uvigo.gal> writes:
>
>> Change the way cdc_ncm_change_mtu hooks into the netdev_ops
>> structure so that changes into usbnet driver_info operations
>> can be respected. Without this, is was not possible to hook
>> into usbnet_set_rx_mode.
>
> Please export usbnet_set_rx_mode instead, so that cdc_ncm_netdev_ops
> can be kept const.
>
I don't see how this would help. From my point of view, the problem is
that cdc_ncm driver should not be setting net_device_ops in the first
place, as that is usbnet.c job. Sadly there seems to be no way currently
for cdc_ncm to be notified of mtu changes. This was the least intrusive
way I could think of to respect usbnet's net_device_ops structure while
keeping cdc_ncm notified of mtu changes. Without this change,
set_rx_mode was not getting called for cdc_ncm even after modifying
struct driver_info.
>
> Bjørn
>
--
Miguel Rodríguez Pérez
Laboratorio de Redes
EE Telecomunicación – Universidade de Vigo
^ permalink raw reply
* [bpf PATCH 0/2] sockmap, syzbot fix error path and RCU fix
From: John Fastabend @ 2018-06-30 13:51 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, kafai
This applies on top of "BPF fixes for sockhash" I just didn't
want to confuse that series yet again by re-ordering/adding
these patches in it
I missed fixing the error path in the sockhash code to align with
supporting socks in multiple maps. Simply checking if the psock is
present does not mean we can decrement the reference count because
it could be part of another map. Fix this by cleaning up the error
path so this situation does not happen.
As far as I know this should be the last fix to the fallout from
relaxing the single map restriction. Sorry about the multiple fixes
but these patches were all written before the initial submission then
converted and I missed this detail. But at least we caught these early
in the net cycle. Will continue reviewing/testing however to see if
we catch anything else.
Also we need one more series to check ESTABLISH state as Eric noted.
That will be sent out shortly just going over the patches once more.
The ESTABLISH/unhash fix is also needed in kTLS.
---
John Fastabend (2):
bpf: sockmap, error path can not release psock in multi-map case
bpf: sockmap, hash table is RCU so readers do not need locks
0 files changed
^ permalink raw reply
* [bpf PATCH 1/2] bpf: sockmap, error path can not release psock in multi-map case
From: John Fastabend @ 2018-06-30 13:51 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, kafai
In-Reply-To: <20180630134148.6395.64795.stgit@john-Precision-Tower-5810>
The current code, in the error path of sock_hash_ctx_update_elem,
checks if the sock has a psock in the user data and if so decrements
the reference count of the psock. However, if the error happens early
in the error path we may have never incremented the psock reference
count and if the psock exists because the sock is in another map then
we may inadvertently decrement the reference count.
Fix this by making the error path only call smap_release_sock if the
error happens after the increment.
Reported-by: syzbot+d464d2c20c717ef5a6a8@syzkaller.appspotmail.com
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
0 files changed
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 4fc2cb1..63fb047 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1896,7 +1896,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
e = kzalloc(sizeof(*e), GFP_ATOMIC | __GFP_NOWARN);
if (!e) {
err = -ENOMEM;
- goto out_progs;
+ goto out_free;
}
}
@@ -2324,7 +2324,12 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
if (err)
goto err;
- /* bpf_map_update_elem() can be called in_irq() */
+ psock = smap_psock_sk(sock);
+ if (unlikely(!psock)) {
+ err = -EINVAL;
+ goto err;
+ }
+
raw_spin_lock_bh(&b->lock);
l_old = lookup_elem_raw(head, hash, key, key_size);
if (l_old && map_flags == BPF_NOEXIST) {
@@ -2342,12 +2347,6 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
goto bucket_err;
}
- psock = smap_psock_sk(sock);
- if (unlikely(!psock)) {
- err = -EINVAL;
- goto bucket_err;
- }
-
rcu_assign_pointer(e->hash_link, l_new);
rcu_assign_pointer(e->htab,
container_of(map, struct bpf_htab, map));
@@ -2370,12 +2369,10 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
raw_spin_unlock_bh(&b->lock);
return 0;
bucket_err:
+ smap_release_sock(psock, sock);
raw_spin_unlock_bh(&b->lock);
err:
kfree(e);
- psock = smap_psock_sk(sock);
- if (psock)
- smap_release_sock(psock, sock);
return err;
}
^ permalink raw reply related
* [bpf PATCH 2/2] bpf: sockmap, hash table is RCU so readers do not need locks
From: John Fastabend @ 2018-06-30 13:51 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, kafai
In-Reply-To: <20180630134148.6395.64795.stgit@john-Precision-Tower-5810>
This removes locking from readers of RCU hash table. Its not
necessary.
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
0 files changed
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 63fb047..12ac10a 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2451,10 +2451,8 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
b = __select_bucket(htab, hash);
head = &b->head;
- raw_spin_lock_bh(&b->lock);
l = lookup_elem_raw(head, hash, key, key_size);
sk = l ? l->sk : NULL;
- raw_spin_unlock_bh(&b->lock);
return sk;
}
^ permalink raw reply related
* Re: [PATCH 0/2 ] cdc_ncm: Handle multicast Ethernet traffic
From: Miguel Rodríguez Pérez @ 2018-06-30 13:43 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-usb, netdev
In-Reply-To: <87muvczcot.fsf@miraculix.mork.no>
On 30/06/18 14:22, Bjørn Mork wrote:
> Miguel Rodríguez Pérez <miguel@det.uvigo.gal> writes:
>
>> Sending again, as the previous try had the wrong subjects. Sorry about that.
>
> No problem really, but please use version numbers so it's easy to see
> which set is most current.
Fine, I'll do so in next revision.
>
> And you didn't address Olivers comment. Why not?
Well, I did, but I forgot to format it as plain text and it didn't reach
the least. I have just resent it.
>
>
> Bjørn
>
--
Miguel Rodríguez Pérez
Laboratorio de Redes
EE Telecomunicación – Universidade de Vigo
^ permalink raw reply
* [bpf PATCH v5 4/4] bpf: sockhash, add release routine
From: John Fastabend @ 2018-06-30 13:17 UTC (permalink / raw)
To: ast, daniel, kafai; +Cc: netdev
In-Reply-To: <20180630131506.17344.7833.stgit@john-Precision-Tower-5810>
Add map_release_uref pointer to hashmap ops. This was dropped when
original sockhash code was ported into bpf-next before initial
commit.
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 60b6e8b..4fc2cb1 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2478,6 +2478,7 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
.map_get_next_key = sock_hash_get_next_key,
.map_update_elem = sock_hash_update_elem,
.map_delete_elem = sock_hash_delete_elem,
+ .map_release_uref = sock_map_release,
};
BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
^ permalink raw reply related
* [bpf PATCH v5 3/4] bpf: sockhash fix omitted bucket lock in sock_close
From: John Fastabend @ 2018-06-30 13:17 UTC (permalink / raw)
To: ast, daniel, kafai; +Cc: netdev
In-Reply-To: <20180630131506.17344.7833.stgit@john-Precision-Tower-5810>
First the sk_callback_lock() was being used to protect both the
sock callback hooks and the psock->maps list. This got overly
convoluted after the addition of sockhash (in sockmap it made
some sense because masp and callbacks were tightly coupled) so
lets split out a specific lock for maps and only use the callback
lock for its intended purpose. This fixes a couple cases where
we missed using maps lock when it was in fact needed. Also this
makes it easier to follow the code because now we can put the
locking closer to the actual code its serializing.
Next, in sock_hash_delete_elem() the pattern was as follows,
sock_hash_delete_elem()
[...]
spin_lock(bucket_lock)
l = lookup_elem_raw()
if (l)
hlist_del_rcu()
write_lock(sk_callback_lock)
.... destroy psock ...
write_unlock(sk_callback_lock)
spin_unlock(bucket_lock)
The ordering is necessary because we only know the {p}sock after
dereferencing the hash table which we can't do unless we have the
bucket lock held. Once we have the bucket lock and the psock element
it is deleted from the hashmap to ensure any other path doing a lookup
will fail. Finally, the refcnt is decremented and if zero the psock
is destroyed.
In parallel with the above (or free'ing the map) a tcp close event
may trigger tcp_close(). Which at the moment omits the bucket lock
altogether (oops!) where the flow looks like this,
bpf_tcp_close()
[...]
write_lock(sk_callback_lock)
for each psock->maps // list of maps this sock is part of
hlist_del_rcu(ref_hash_node);
.... destroy psock ...
write_unlock(sk_callback_lock)
Obviously, and demonstrated by syzbot, this is broken because
we can have multiple threads deleting entries via hlist_del_rcu().
To fix this we might be tempted to wrap the hlist operation in a
bucket lock but that would create a lock inversion problem. In
summary to follow locking rules the psocks maps list needs the
sk_callback_lock (after this patch maps_lock) but we need the bucket
lock to do the hlist_del_rcu.
To resolve the lock inversion problem pop the head of the maps list
repeatedly and remove the reference until no more are left. If a
delete happens in parallel from the BPF API that is OK as well because
it will do a similar action, lookup the lock in the map/hash, delete
it from the map/hash, and dec the refcnt. We check for this case
before doing a destroy on the psock to ensure we don't have two
threads tearing down a psock. The new logic is as follows,
bpf_tcp_close()
e = psock_map_pop(psock->maps) // done with map lock
bucket_lock() // lock hash list bucket
l = lookup_elem_raw(head, hash, key, key_size);
if (l) {
//only get here if elmnt was not already removed
hlist_del_rcu()
... destroy psock...
}
bucket_unlock()
And finally for all the above to work add missing locking around map
operations per above. Then add RCU annotations and use
rcu_dereference/rcu_assign_pointer to manage values relying on RCU so
that the object is not free'd from sock_hash_free() while it is being
referenced in bpf_tcp_close().
Reported-by: syzbot+0ce137753c78f7b6acc1@syzkaller.appspotmail.com
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 145 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 96 insertions(+), 49 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 5adff4b..60b6e8b 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -72,6 +72,7 @@ struct bpf_htab {
u32 n_buckets;
u32 elem_size;
struct bpf_sock_progs progs;
+ struct rcu_head rcu;
};
struct htab_elem {
@@ -89,8 +90,8 @@ enum smap_psock_state {
struct smap_psock_map_entry {
struct list_head list;
struct sock **entry;
- struct htab_elem *hash_link;
- struct bpf_htab *htab;
+ struct htab_elem __rcu *hash_link;
+ struct bpf_htab __rcu *htab;
};
struct smap_psock {
@@ -120,6 +121,7 @@ struct smap_psock {
struct bpf_prog *bpf_parse;
struct bpf_prog *bpf_verdict;
struct list_head maps;
+ spinlock_t maps_lock;
/* Back reference used when sock callback trigger sockmap operations */
struct sock *sock;
@@ -258,16 +260,54 @@ static void bpf_tcp_release(struct sock *sk)
rcu_read_unlock();
}
+static struct htab_elem *lookup_elem_raw(struct hlist_head *head,
+ u32 hash, void *key, u32 key_size)
+{
+ struct htab_elem *l;
+
+ hlist_for_each_entry_rcu(l, head, hash_node) {
+ if (l->hash == hash && !memcmp(&l->key, key, key_size))
+ return l;
+ }
+
+ return NULL;
+}
+
+static inline struct bucket *__select_bucket(struct bpf_htab *htab, u32 hash)
+{
+ return &htab->buckets[hash & (htab->n_buckets - 1)];
+}
+
+static inline struct hlist_head *select_bucket(struct bpf_htab *htab, u32 hash)
+{
+ return &__select_bucket(htab, hash)->head;
+}
+
static void free_htab_elem(struct bpf_htab *htab, struct htab_elem *l)
{
atomic_dec(&htab->count);
kfree_rcu(l, rcu);
}
+static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,
+ struct smap_psock *psock)
+{
+ struct smap_psock_map_entry *e;
+
+ spin_lock_bh(&psock->maps_lock);
+ e = list_first_entry_or_null(&psock->maps,
+ struct smap_psock_map_entry,
+ list);
+ if (e)
+ list_del(&e->list);
+ spin_unlock_bh(&psock->maps_lock);
+ return e;
+}
+
static void bpf_tcp_close(struct sock *sk, long timeout)
{
void (*close_fun)(struct sock *sk, long timeout);
- struct smap_psock_map_entry *e, *tmp;
+ struct smap_psock_map_entry *e;
struct sk_msg_buff *md, *mtmp;
struct smap_psock *psock;
struct sock *osk;
@@ -286,7 +326,6 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
*/
close_fun = psock->save_close;
- write_lock_bh(&sk->sk_callback_lock);
if (psock->cork) {
free_start_sg(psock->sock, psock->cork);
kfree(psock->cork);
@@ -299,20 +338,38 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
kfree(md);
}
- list_for_each_entry_safe(e, tmp, &psock->maps, list) {
+ e = psock_map_pop(sk, psock);
+ while (e) {
if (e->entry) {
osk = cmpxchg(e->entry, sk, NULL);
if (osk == sk) {
- list_del(&e->list);
smap_release_sock(psock, sk);
}
} else {
- hlist_del_rcu(&e->hash_link->hash_node);
- smap_release_sock(psock, e->hash_link->sk);
- free_htab_elem(e->htab, e->hash_link);
+ struct htab_elem *link = rcu_dereference(e->hash_link);
+ struct bpf_htab *htab = rcu_dereference(e->htab);
+ struct hlist_head *head;
+ struct htab_elem *l;
+ struct bucket *b;
+
+ b = __select_bucket(htab, link->hash);
+ head = &b->head;
+ raw_spin_lock_bh(&b->lock);
+ l = lookup_elem_raw(head,
+ link->hash, link->key,
+ htab->map.key_size);
+ /* If another thread deleted this object skip deletion.
+ * The refcnt on psock may or may not be zero.
+ */
+ if (l) {
+ hlist_del_rcu(&link->hash_node);
+ smap_release_sock(psock, link->sk);
+ free_htab_elem(htab, link);
+ }
+ raw_spin_unlock_bh(&b->lock);
}
+ e = psock_map_pop(sk, psock);
}
- write_unlock_bh(&sk->sk_callback_lock);
rcu_read_unlock();
close_fun(sk, timeout);
}
@@ -1395,7 +1452,9 @@ static void smap_release_sock(struct smap_psock *psock, struct sock *sock)
{
if (refcount_dec_and_test(&psock->refcnt)) {
tcp_cleanup_ulp(sock);
+ write_lock_bh(&sock->sk_callback_lock);
smap_stop_sock(psock, sock);
+ write_unlock_bh(&sock->sk_callback_lock);
clear_bit(SMAP_TX_RUNNING, &psock->state);
rcu_assign_sk_user_data(sock, NULL);
call_rcu_sched(&psock->rcu, smap_destroy_psock);
@@ -1546,6 +1605,7 @@ static struct smap_psock *smap_init_psock(struct sock *sock, int node)
INIT_LIST_HEAD(&psock->maps);
INIT_LIST_HEAD(&psock->ingress);
refcount_set(&psock->refcnt, 1);
+ spin_lock_init(&psock->maps_lock);
rcu_assign_sk_user_data(sock, psock);
sock_hold(sock);
@@ -1607,10 +1667,12 @@ static void smap_list_map_remove(struct smap_psock *psock,
{
struct smap_psock_map_entry *e, *tmp;
+ spin_lock_bh(&psock->maps_lock);
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
if (e->entry == entry)
list_del(&e->list);
}
+ spin_unlock_bh(&psock->maps_lock);
}
static void smap_list_hash_remove(struct smap_psock *psock,
@@ -1618,12 +1680,14 @@ static void smap_list_hash_remove(struct smap_psock *psock,
{
struct smap_psock_map_entry *e, *tmp;
+ spin_lock_bh(&psock->maps_lock);
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
- struct htab_elem *c = e->hash_link;
+ struct htab_elem *c = rcu_dereference(e->hash_link);
if (c == hash_link)
list_del(&e->list);
}
+ spin_unlock_bh(&psock->maps_lock);
}
static void sock_map_free(struct bpf_map *map)
@@ -1649,7 +1713,6 @@ static void sock_map_free(struct bpf_map *map)
if (!sock)
continue;
- write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
/* This check handles a racing sock event that can get the
* sk_callback_lock before this case but after xchg happens
@@ -1660,7 +1723,6 @@ static void sock_map_free(struct bpf_map *map)
smap_list_map_remove(psock, &stab->sock_map[i]);
smap_release_sock(psock, sock);
}
- write_unlock_bh(&sock->sk_callback_lock);
}
rcu_read_unlock();
@@ -1709,7 +1771,6 @@ static int sock_map_delete_elem(struct bpf_map *map, void *key)
if (!sock)
return -EINVAL;
- write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
if (!psock)
goto out;
@@ -1719,7 +1780,6 @@ static int sock_map_delete_elem(struct bpf_map *map, void *key)
smap_list_map_remove(psock, &stab->sock_map[k]);
smap_release_sock(psock, sock);
out:
- write_unlock_bh(&sock->sk_callback_lock);
return 0;
}
@@ -1800,7 +1860,6 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
}
}
- write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
/* 2. Do not allow inheriting programs if psock exists and has
@@ -1857,7 +1916,9 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
if (err)
goto out_free;
smap_init_progs(psock, verdict, parse);
+ write_lock_bh(&sock->sk_callback_lock);
smap_start_sock(psock, sock);
+ write_unlock_bh(&sock->sk_callback_lock);
}
/* 4. Place psock in sockmap for use and stop any programs on
@@ -1867,9 +1928,10 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
*/
if (map_link) {
e->entry = map_link;
+ spin_lock_bh(&psock->maps_lock);
list_add_tail(&e->list, &psock->maps);
+ spin_unlock_bh(&psock->maps_lock);
}
- write_unlock_bh(&sock->sk_callback_lock);
return err;
out_free:
smap_release_sock(psock, sock);
@@ -1880,7 +1942,6 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
}
if (tx_msg)
bpf_prog_put(tx_msg);
- write_unlock_bh(&sock->sk_callback_lock);
kfree(e);
return err;
}
@@ -1917,10 +1978,8 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
if (osock) {
struct smap_psock *opsock = smap_psock_sk(osock);
- write_lock_bh(&osock->sk_callback_lock);
smap_list_map_remove(opsock, &stab->sock_map[i]);
smap_release_sock(opsock, osock);
- write_unlock_bh(&osock->sk_callback_lock);
}
out:
return err;
@@ -2091,14 +2150,13 @@ static struct bpf_map *sock_hash_alloc(union bpf_attr *attr)
return ERR_PTR(err);
}
-static inline struct bucket *__select_bucket(struct bpf_htab *htab, u32 hash)
+static void __bpf_htab_free(struct rcu_head *rcu)
{
- return &htab->buckets[hash & (htab->n_buckets - 1)];
-}
+ struct bpf_htab *htab;
-static inline struct hlist_head *select_bucket(struct bpf_htab *htab, u32 hash)
-{
- return &__select_bucket(htab, hash)->head;
+ htab = container_of(rcu, struct bpf_htab, rcu);
+ bpf_map_area_free(htab->buckets);
+ kfree(htab);
}
static void sock_hash_free(struct bpf_map *map)
@@ -2117,16 +2175,18 @@ static void sock_hash_free(struct bpf_map *map)
*/
rcu_read_lock();
for (i = 0; i < htab->n_buckets; i++) {
- struct hlist_head *head = select_bucket(htab, i);
+ struct bucket *b = __select_bucket(htab, i);
+ struct hlist_head *head;
struct hlist_node *n;
struct htab_elem *l;
+ raw_spin_lock_bh(&b->lock);
+ head = &b->head;
hlist_for_each_entry_safe(l, n, head, hash_node) {
struct sock *sock = l->sk;
struct smap_psock *psock;
hlist_del_rcu(&l->hash_node);
- write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
/* This check handles a racing sock event that can get
* the sk_callback_lock before this case but after xchg
@@ -2137,13 +2197,12 @@ static void sock_hash_free(struct bpf_map *map)
smap_list_hash_remove(psock, l);
smap_release_sock(psock, sock);
}
- write_unlock_bh(&sock->sk_callback_lock);
- kfree(l);
+ free_htab_elem(htab, l);
}
+ raw_spin_unlock_bh(&b->lock);
}
rcu_read_unlock();
- bpf_map_area_free(htab->buckets);
- kfree(htab);
+ call_rcu(&htab->rcu, __bpf_htab_free);
}
static struct htab_elem *alloc_sock_hash_elem(struct bpf_htab *htab,
@@ -2170,19 +2229,6 @@ static struct htab_elem *alloc_sock_hash_elem(struct bpf_htab *htab,
return l_new;
}
-static struct htab_elem *lookup_elem_raw(struct hlist_head *head,
- u32 hash, void *key, u32 key_size)
-{
- struct htab_elem *l;
-
- hlist_for_each_entry_rcu(l, head, hash_node) {
- if (l->hash == hash && !memcmp(&l->key, key, key_size))
- return l;
- }
-
- return NULL;
-}
-
static inline u32 htab_map_hash(const void *key, u32 key_len)
{
return jhash(key, key_len, 0);
@@ -2302,9 +2348,12 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
goto bucket_err;
}
- e->hash_link = l_new;
- e->htab = container_of(map, struct bpf_htab, map);
+ rcu_assign_pointer(e->hash_link, l_new);
+ rcu_assign_pointer(e->htab,
+ container_of(map, struct bpf_htab, map));
+ spin_lock_bh(&psock->maps_lock);
list_add_tail(&e->list, &psock->maps);
+ spin_unlock_bh(&psock->maps_lock);
/* add new element to the head of the list, so that
* concurrent search will find it before old elem
@@ -2374,7 +2423,6 @@ static int sock_hash_delete_elem(struct bpf_map *map, void *key)
struct smap_psock *psock;
hlist_del_rcu(&l->hash_node);
- write_lock_bh(&sock->sk_callback_lock);
psock = smap_psock_sk(sock);
/* This check handles a racing sock event that can get the
* sk_callback_lock before this case but after xchg happens
@@ -2385,7 +2433,6 @@ static int sock_hash_delete_elem(struct bpf_map *map, void *key)
smap_list_hash_remove(psock, l);
smap_release_sock(psock, sock);
}
- write_unlock_bh(&sock->sk_callback_lock);
free_htab_elem(htab, l);
ret = 0;
}
^ permalink raw reply related
* [bpf PATCH v5 2/4] bpf: sockmap, fix smap_list_map_remove when psock is in many maps
From: John Fastabend @ 2018-06-30 13:17 UTC (permalink / raw)
To: ast, daniel, kafai; +Cc: netdev
In-Reply-To: <20180630131506.17344.7833.stgit@john-Precision-Tower-5810>
If a hashmap is free'd with open socks it removes the reference to
the hash entry from the psock. If that is the last reference to the
psock then it will also be free'd by the reference counting logic.
However the current logic that removes the hash reference from the
list of references is broken. In smap_list_remove() we first check
if the sockmap entry matches and then check if the hashmap entry
matches. But, the sockmap entry sill always match because its NULL in
this case which causes the first entry to be removed from the list.
If this is always the "right" entry (because the user adds/removes
entries in order) then everything is OK but otherwise a subsequent
bpf_tcp_close() may reference a free'd object.
To fix this create two list handlers one for sockmap and one for
sockhash.
Reported-by: syzbot+0ce137753c78f7b6acc1@syzkaller.appspotmail.com
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index d7fd17a..5adff4b 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1602,17 +1602,27 @@ static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
return ERR_PTR(err);
}
-static void smap_list_remove(struct smap_psock *psock,
- struct sock **entry,
- struct htab_elem *hash_link)
+static void smap_list_map_remove(struct smap_psock *psock,
+ struct sock **entry)
{
struct smap_psock_map_entry *e, *tmp;
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
- if (e->entry == entry || e->hash_link == hash_link) {
+ if (e->entry == entry)
+ list_del(&e->list);
+ }
+}
+
+static void smap_list_hash_remove(struct smap_psock *psock,
+ struct htab_elem *hash_link)
+{
+ struct smap_psock_map_entry *e, *tmp;
+
+ list_for_each_entry_safe(e, tmp, &psock->maps, list) {
+ struct htab_elem *c = e->hash_link;
+
+ if (c == hash_link)
list_del(&e->list);
- break;
- }
}
}
@@ -1647,7 +1657,7 @@ static void sock_map_free(struct bpf_map *map)
* to be null and queued for garbage collection.
*/
if (likely(psock)) {
- smap_list_remove(psock, &stab->sock_map[i], NULL);
+ smap_list_map_remove(psock, &stab->sock_map[i]);
smap_release_sock(psock, sock);
}
write_unlock_bh(&sock->sk_callback_lock);
@@ -1706,7 +1716,7 @@ static int sock_map_delete_elem(struct bpf_map *map, void *key)
if (psock->bpf_parse)
smap_stop_sock(psock, sock);
- smap_list_remove(psock, &stab->sock_map[k], NULL);
+ smap_list_map_remove(psock, &stab->sock_map[k]);
smap_release_sock(psock, sock);
out:
write_unlock_bh(&sock->sk_callback_lock);
@@ -1908,7 +1918,7 @@ 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);
- smap_list_remove(opsock, &stab->sock_map[i], NULL);
+ smap_list_map_remove(opsock, &stab->sock_map[i]);
smap_release_sock(opsock, osock);
write_unlock_bh(&osock->sk_callback_lock);
}
@@ -2124,7 +2134,7 @@ static void sock_hash_free(struct bpf_map *map)
* (psock) to be null and queued for garbage collection.
*/
if (likely(psock)) {
- smap_list_remove(psock, NULL, l);
+ smap_list_hash_remove(psock, l);
smap_release_sock(psock, sock);
}
write_unlock_bh(&sock->sk_callback_lock);
@@ -2304,7 +2314,7 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
psock = smap_psock_sk(l_old->sk);
hlist_del_rcu(&l_old->hash_node);
- smap_list_remove(psock, NULL, l_old);
+ smap_list_hash_remove(psock, l_old);
smap_release_sock(psock, l_old->sk);
free_htab_elem(htab, l_old);
}
@@ -2372,7 +2382,7 @@ static int sock_hash_delete_elem(struct bpf_map *map, void *key)
* to be null and queued for garbage collection.
*/
if (likely(psock)) {
- smap_list_remove(psock, NULL, l);
+ smap_list_hash_remove(psock, l);
smap_release_sock(psock, sock);
}
write_unlock_bh(&sock->sk_callback_lock);
^ permalink raw reply related
* [bpf PATCH v5 1/4] bpf: sockmap, fix crash when ipv6 sock is added
From: John Fastabend @ 2018-06-30 13:17 UTC (permalink / raw)
To: ast, daniel, kafai; +Cc: netdev
In-Reply-To: <20180630131506.17344.7833.stgit@john-Precision-Tower-5810>
This fixes a crash where we assign tcp_prot to IPv6 sockets instead
of tcpv6_prot.
Previously we overwrote the sk->prot field with tcp_prot even in the
AF_INET6 case. This patch ensures the correct tcp_prot and tcpv6_prot
are used.
Tested with 'netserver -6' and 'netperf -H [IPv6]' as well as
'netperf -H [IPv4]'. The ESTABLISHED check resolves the previously
crashing case here.
Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Reported-by: syzbot+5c063698bdbfac19f363@syzkaller.appspotmail.com
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Wei Wang <weiwan@google.com>
---
kernel/bpf/sockmap.c | 58 +++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 48 insertions(+), 10 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 52a91d8..d7fd17a 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -140,6 +140,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
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 void bpf_tcp_close(struct sock *sk, long timeout);
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
{
@@ -161,7 +162,42 @@ static bool bpf_tcp_stream_read(const struct sock *sk)
return !empty;
}
-static struct proto tcp_bpf_proto;
+enum {
+ SOCKMAP_IPV4,
+ SOCKMAP_IPV6,
+ SOCKMAP_NUM_PROTS,
+};
+
+enum {
+ SOCKMAP_BASE,
+ SOCKMAP_TX,
+ SOCKMAP_NUM_CONFIGS,
+};
+
+static struct proto *saved_tcpv6_prot __read_mostly;
+static DEFINE_SPINLOCK(tcpv6_prot_lock);
+static struct proto bpf_tcp_prots[SOCKMAP_NUM_PROTS][SOCKMAP_NUM_CONFIGS];
+static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
+ struct proto *base)
+{
+ prot[SOCKMAP_BASE] = *base;
+ prot[SOCKMAP_BASE].close = bpf_tcp_close;
+ prot[SOCKMAP_BASE].recvmsg = bpf_tcp_recvmsg;
+ prot[SOCKMAP_BASE].stream_memory_read = bpf_tcp_stream_read;
+
+ prot[SOCKMAP_TX] = prot[SOCKMAP_BASE];
+ prot[SOCKMAP_TX].sendmsg = bpf_tcp_sendmsg;
+ prot[SOCKMAP_TX].sendpage = bpf_tcp_sendpage;
+}
+
+static void update_sk_prot(struct sock *sk, struct smap_psock *psock)
+{
+ int family = sk->sk_family == AF_INET6 ? SOCKMAP_IPV6 : SOCKMAP_IPV4;
+ int conf = psock->bpf_tx_msg ? SOCKMAP_TX : SOCKMAP_BASE;
+
+ sk->sk_prot = &bpf_tcp_prots[family][conf];
+}
+
static int bpf_tcp_init(struct sock *sk)
{
struct smap_psock *psock;
@@ -181,14 +217,17 @@ 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;
- tcp_bpf_proto.recvmsg = bpf_tcp_recvmsg;
- tcp_bpf_proto.stream_memory_read = bpf_tcp_stream_read;
+ /* Build IPv6 sockmap whenever the address of tcpv6_prot changes */
+ if (sk->sk_family == AF_INET6 &&
+ unlikely(sk->sk_prot != smp_load_acquire(&saved_tcpv6_prot))) {
+ spin_lock_bh(&tcpv6_prot_lock);
+ if (likely(sk->sk_prot != saved_tcpv6_prot)) {
+ build_protos(bpf_tcp_prots[SOCKMAP_IPV6], sk->sk_prot);
+ smp_store_release(&saved_tcpv6_prot, sk->sk_prot);
+ }
+ spin_unlock_bh(&tcpv6_prot_lock);
}
-
- sk->sk_prot = &tcp_bpf_proto;
+ update_sk_prot(sk, psock);
rcu_read_unlock();
return 0;
}
@@ -1111,8 +1150,7 @@ static void bpf_tcp_msg_add(struct smap_psock *psock,
static int bpf_tcp_ulp_register(void)
{
- tcp_bpf_proto = tcp_prot;
- tcp_bpf_proto.close = bpf_tcp_close;
+ build_protos(bpf_tcp_prots[SOCKMAP_IPV4], &tcp_prot);
/* 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.
^ permalink raw reply related
* [bpf PATCH v5 0/4] BPF fixes for sockhash
From: John Fastabend @ 2018-06-30 13:17 UTC (permalink / raw)
To: ast, daniel, kafai; +Cc: netdev
This addresses two syzbot issues that lead to identifying (by Eric and
Wei) a class of bugs where we don't correctly check for IPv4/v6
sockets and their associated state. The second issue was a locking
omission in sockhash.
The first patch addresses IPv6 socks and fixing an error where
sockhash would overwrite the prot pointer with IPv4 prot. To fix
this build similar solution to TLS ULP. Although we continue to
allow socks in all states not just ESTABLISH in this patch set
because as Martin points out there should be no issue with this
on the sockmap ULP because we don't use the ctx in this code. Once
multiple ULPs coexist we may need to revisit this. However we
can do this in *next trees.
The other issue syzbot found that the tcp_close() handler missed
locking the hash bucket lock which could result in corrupting the
sockhash bucket list if delete and close ran at the same time.
And also the smap_list_remove() routine was not working correctly
at all. This was not caught in my testing because in general my
tests (to date at least lets add some more robust selftest in
bpf-next) do things in the "expected" order, create map, add socks,
delete socks, then tear down maps. The tests we have that do the
ops out of this order where only working on single maps not multi-
maps so we never saw the issue. Thanks syzbot. The fix is to
restructure the tcp_close() lock handling. And fix the obvious
bug in smap_list_remove().
Finally, during review I noticed the release handler was omitted
from the upstream code (patch 4) due to an incorrect merge conflict
fix when I ported the code to latest bpf-next before submitting.
This would leave references to the map around if the user never
closes the map.
v3: rework patches, dropping ESTABLISH check and adding rcu
annotation along with the smap_list_remove fix
v4: missed one more case where maps was being accessed without
the sk_callback_lock, spoted by Martin as well.
v5: changed to use a specific lock for maps and reduced callback
lock so that it is only used to gaurd sk callbacks. I think
this makes the logic a bit cleaner and avoids confusion
ovoer what each lock is doing.
Also big thanks to Martin for thorough review he caught at least
one case where I missed a rcu_call().
---
John Fastabend (4):
bpf: sockmap, fix crash when ipv6 sock is added
bpf: sockmap, fix smap_list_map_remove when psock is in many maps
bpf: sockhash fix omitted bucket lock in sock_close
bpf: sockhash, add release routine
kernel/bpf/sockmap.c | 236 +++++++++++++++++++++++++++++++++++---------------
1 file changed, 166 insertions(+), 70 deletions(-)
^ permalink raw reply
* Re: [PATCH net] net: fib_rules: bring back rule_exists to match rule during add
From: David Miller @ 2018-06-30 13:11 UTC (permalink / raw)
To: roopa; +Cc: netdev, dsa
In-Reply-To: <1530307935-6652-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Fri, 29 Jun 2018 14:32:15 -0700
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> After commit f9d4b0c1e969 ("fib_rules: move common handling of newrule
> delrule msgs into fib_nl2rule"), rule_exists got replaced by rule_find
> for existing rule lookup in both the add and del paths. While this
> is good for the delete path, it solves a few problems but opens up
> a few invalid key matches in the add path.
>
> $ip -4 rule add table main tos 10 fwmark 1
> $ip -4 rule add table main tos 10
> RTNETLINK answers: File exists
>
> The problem here is rule_find does not check if the key masks in
> the new and old rule are the same and hence ends up matching a more
> secific rule. Rule key masks cannot be easily compared today without
> an elaborate if-else block. Its best to introduce key masks for easier
> and accurate rule comparison in the future. Until then, due to fear of
> regressions this patch re-introduces older loose rule_exists during add.
> Also fixes both rule_exists and rule_find to cover missing attributes.
>
> Fixes: f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs into fib_nl2rule")
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Applied, thanks for resolving all of these issues.
^ permalink raw reply
* Re: [PATCH net-next 00/13] mlxsw: Add resource scale tests
From: David Miller @ 2018-06-30 13:06 UTC (permalink / raw)
To: petrm; +Cc: netdev, linux-kselftest, jiri, idosch, shuah
In-Reply-To: <cover.1530319109.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Sat, 30 Jun 2018 02:44:23 +0200
> There are a number of tests that check features of the Linux networking
> stack. By running them on suitable interfaces, one can exercise the
> mlxsw offloading code. However none of these tests attempts to push
> mlxsw to the limits supported by the ASIC.
>
> As an additional wrinkle, the "limits supported by the ASIC" themselves
> may not be a set of fixed numbers, but rather depend on a profile that
> determines how the ASIC resources are allocated for different purposes.
>
> This patchset introduces several tests that verify capability of mlxsw
> to offload amounts of routes, flower rules, and mirroring sessions that
> match predicted ASIC capacity, at different configuration profiles.
> Additionally they verify that amounts exceeding the predicted capacity
> can *not* be offloaded.
...
Good stuff, series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next 4/5] net/sched: flower: Dump the ethertype encapsulated in vlan
From: kbuild test robot @ 2018-06-30 12:54 UTC (permalink / raw)
To: Jianbo Liu
Cc: kbuild-all, netdev, davem, jiri, Jianbo Liu, Jamal Hadi Salim,
Cong Wang
In-Reply-To: <20180630095317.5691-5-jianbol@mellanox.com>
Hi Jianbo,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jianbo-Liu/net-flow_dissector-Save-vlan-ethertype-from-headers/20180630-180158
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:544:15: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
net/sched/cls_flower.c:545:16: sparse: cast to restricted __be32
include/linux/slab.h:631:13: sparse: undefined identifier '__builtin_mul_overflow'
include/linux/slab.h:631:13: sparse: not a function <noident>
>> net/sched/cls_flower.c:1317:70: sparse: incorrect type in argument 3 (different base types) @@ expected unsigned short [unsigned] [usertype] value @@ got short [unsigned] [usertype] value @@
net/sched/cls_flower.c:1317:70: expected unsigned short [unsigned] [usertype] value
net/sched/cls_flower.c:1317:70: got restricted __be16 [usertype] n_proto
include/linux/slab.h:631:13: sparse: call with no type!
vim +1317 net/sched/cls_flower.c
1264
1265 static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
1266 struct sk_buff *skb, struct tcmsg *t)
1267 {
1268 struct cls_fl_filter *f = fh;
1269 struct nlattr *nest;
1270 struct fl_flow_key *key, *mask;
1271
1272 if (!f)
1273 return skb->len;
1274
1275 t->tcm_handle = f->handle;
1276
1277 nest = nla_nest_start(skb, TCA_OPTIONS);
1278 if (!nest)
1279 goto nla_put_failure;
1280
1281 if (f->res.classid &&
1282 nla_put_u32(skb, TCA_FLOWER_CLASSID, f->res.classid))
1283 goto nla_put_failure;
1284
1285 key = &f->key;
1286 mask = &f->mask->key;
1287
1288 if (mask->indev_ifindex) {
1289 struct net_device *dev;
1290
1291 dev = __dev_get_by_index(net, key->indev_ifindex);
1292 if (dev && nla_put_string(skb, TCA_FLOWER_INDEV, dev->name))
1293 goto nla_put_failure;
1294 }
1295
1296 if (!tc_skip_hw(f->flags))
1297 fl_hw_update_stats(tp, f);
1298
1299 if (fl_dump_key_val(skb, key->eth.dst, TCA_FLOWER_KEY_ETH_DST,
1300 mask->eth.dst, TCA_FLOWER_KEY_ETH_DST_MASK,
1301 sizeof(key->eth.dst)) ||
1302 fl_dump_key_val(skb, key->eth.src, TCA_FLOWER_KEY_ETH_SRC,
1303 mask->eth.src, TCA_FLOWER_KEY_ETH_SRC_MASK,
1304 sizeof(key->eth.src)) ||
1305 fl_dump_key_val(skb, &key->basic.n_proto, TCA_FLOWER_KEY_ETH_TYPE,
1306 &mask->basic.n_proto, TCA_FLOWER_UNSPEC,
1307 sizeof(key->basic.n_proto)))
1308 goto nla_put_failure;
1309
1310 if (fl_dump_key_mpls(skb, &key->mpls, &mask->mpls))
1311 goto nla_put_failure;
1312
1313 if (fl_dump_key_vlan(skb, &key->vlan, &mask->vlan))
1314 goto nla_put_failure;
1315
1316 if (mask->vlan.vlan_tpid &&
> 1317 nla_put_u16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, key->basic.n_proto))
1318 goto nla_put_failure;
1319
1320 if ((key->basic.n_proto == htons(ETH_P_IP) ||
1321 key->basic.n_proto == htons(ETH_P_IPV6)) &&
1322 (fl_dump_key_val(skb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO,
1323 &mask->basic.ip_proto, TCA_FLOWER_UNSPEC,
1324 sizeof(key->basic.ip_proto)) ||
1325 fl_dump_key_ip(skb, &key->ip, &mask->ip)))
1326 goto nla_put_failure;
1327
1328 if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
1329 (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC,
1330 &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,
1331 sizeof(key->ipv4.src)) ||
1332 fl_dump_key_val(skb, &key->ipv4.dst, TCA_FLOWER_KEY_IPV4_DST,
1333 &mask->ipv4.dst, TCA_FLOWER_KEY_IPV4_DST_MASK,
1334 sizeof(key->ipv4.dst))))
1335 goto nla_put_failure;
1336 else if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS &&
1337 (fl_dump_key_val(skb, &key->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC,
1338 &mask->ipv6.src, TCA_FLOWER_KEY_IPV6_SRC_MASK,
1339 sizeof(key->ipv6.src)) ||
1340 fl_dump_key_val(skb, &key->ipv6.dst, TCA_FLOWER_KEY_IPV6_DST,
1341 &mask->ipv6.dst, TCA_FLOWER_KEY_IPV6_DST_MASK,
1342 sizeof(key->ipv6.dst))))
1343 goto nla_put_failure;
1344
1345 if (key->basic.ip_proto == IPPROTO_TCP &&
1346 (fl_dump_key_val(skb, &key->tp.src, TCA_FLOWER_KEY_TCP_SRC,
1347 &mask->tp.src, TCA_FLOWER_KEY_TCP_SRC_MASK,
1348 sizeof(key->tp.src)) ||
1349 fl_dump_key_val(skb, &key->tp.dst, TCA_FLOWER_KEY_TCP_DST,
1350 &mask->tp.dst, TCA_FLOWER_KEY_TCP_DST_MASK,
1351 sizeof(key->tp.dst)) ||
1352 fl_dump_key_val(skb, &key->tcp.flags, TCA_FLOWER_KEY_TCP_FLAGS,
1353 &mask->tcp.flags, TCA_FLOWER_KEY_TCP_FLAGS_MASK,
1354 sizeof(key->tcp.flags))))
1355 goto nla_put_failure;
1356 else if (key->basic.ip_proto == IPPROTO_UDP &&
1357 (fl_dump_key_val(skb, &key->tp.src, TCA_FLOWER_KEY_UDP_SRC,
1358 &mask->tp.src, TCA_FLOWER_KEY_UDP_SRC_MASK,
1359 sizeof(key->tp.src)) ||
1360 fl_dump_key_val(skb, &key->tp.dst, TCA_FLOWER_KEY_UDP_DST,
1361 &mask->tp.dst, TCA_FLOWER_KEY_UDP_DST_MASK,
1362 sizeof(key->tp.dst))))
1363 goto nla_put_failure;
1364 else if (key->basic.ip_proto == IPPROTO_SCTP &&
1365 (fl_dump_key_val(skb, &key->tp.src, TCA_FLOWER_KEY_SCTP_SRC,
1366 &mask->tp.src, TCA_FLOWER_KEY_SCTP_SRC_MASK,
1367 sizeof(key->tp.src)) ||
1368 fl_dump_key_val(skb, &key->tp.dst, TCA_FLOWER_KEY_SCTP_DST,
1369 &mask->tp.dst, TCA_FLOWER_KEY_SCTP_DST_MASK,
1370 sizeof(key->tp.dst))))
1371 goto nla_put_failure;
1372 else if (key->basic.n_proto == htons(ETH_P_IP) &&
1373 key->basic.ip_proto == IPPROTO_ICMP &&
1374 (fl_dump_key_val(skb, &key->icmp.type,
1375 TCA_FLOWER_KEY_ICMPV4_TYPE, &mask->icmp.type,
1376 TCA_FLOWER_KEY_ICMPV4_TYPE_MASK,
1377 sizeof(key->icmp.type)) ||
1378 fl_dump_key_val(skb, &key->icmp.code,
1379 TCA_FLOWER_KEY_ICMPV4_CODE, &mask->icmp.code,
1380 TCA_FLOWER_KEY_ICMPV4_CODE_MASK,
1381 sizeof(key->icmp.code))))
1382 goto nla_put_failure;
1383 else if (key->basic.n_proto == htons(ETH_P_IPV6) &&
1384 key->basic.ip_proto == IPPROTO_ICMPV6 &&
1385 (fl_dump_key_val(skb, &key->icmp.type,
1386 TCA_FLOWER_KEY_ICMPV6_TYPE, &mask->icmp.type,
1387 TCA_FLOWER_KEY_ICMPV6_TYPE_MASK,
1388 sizeof(key->icmp.type)) ||
1389 fl_dump_key_val(skb, &key->icmp.code,
1390 TCA_FLOWER_KEY_ICMPV6_CODE, &mask->icmp.code,
1391 TCA_FLOWER_KEY_ICMPV6_CODE_MASK,
1392 sizeof(key->icmp.code))))
1393 goto nla_put_failure;
1394 else if ((key->basic.n_proto == htons(ETH_P_ARP) ||
1395 key->basic.n_proto == htons(ETH_P_RARP)) &&
1396 (fl_dump_key_val(skb, &key->arp.sip,
1397 TCA_FLOWER_KEY_ARP_SIP, &mask->arp.sip,
1398 TCA_FLOWER_KEY_ARP_SIP_MASK,
1399 sizeof(key->arp.sip)) ||
1400 fl_dump_key_val(skb, &key->arp.tip,
1401 TCA_FLOWER_KEY_ARP_TIP, &mask->arp.tip,
1402 TCA_FLOWER_KEY_ARP_TIP_MASK,
1403 sizeof(key->arp.tip)) ||
1404 fl_dump_key_val(skb, &key->arp.op,
1405 TCA_FLOWER_KEY_ARP_OP, &mask->arp.op,
1406 TCA_FLOWER_KEY_ARP_OP_MASK,
1407 sizeof(key->arp.op)) ||
1408 fl_dump_key_val(skb, key->arp.sha, TCA_FLOWER_KEY_ARP_SHA,
1409 mask->arp.sha, TCA_FLOWER_KEY_ARP_SHA_MASK,
1410 sizeof(key->arp.sha)) ||
1411 fl_dump_key_val(skb, key->arp.tha, TCA_FLOWER_KEY_ARP_THA,
1412 mask->arp.tha, TCA_FLOWER_KEY_ARP_THA_MASK,
1413 sizeof(key->arp.tha))))
1414 goto nla_put_failure;
1415
1416 if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
1417 (fl_dump_key_val(skb, &key->enc_ipv4.src,
1418 TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,
1419 TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK,
1420 sizeof(key->enc_ipv4.src)) ||
1421 fl_dump_key_val(skb, &key->enc_ipv4.dst,
1422 TCA_FLOWER_KEY_ENC_IPV4_DST, &mask->enc_ipv4.dst,
1423 TCA_FLOWER_KEY_ENC_IPV4_DST_MASK,
1424 sizeof(key->enc_ipv4.dst))))
1425 goto nla_put_failure;
1426 else if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS &&
1427 (fl_dump_key_val(skb, &key->enc_ipv6.src,
1428 TCA_FLOWER_KEY_ENC_IPV6_SRC, &mask->enc_ipv6.src,
1429 TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK,
1430 sizeof(key->enc_ipv6.src)) ||
1431 fl_dump_key_val(skb, &key->enc_ipv6.dst,
1432 TCA_FLOWER_KEY_ENC_IPV6_DST,
1433 &mask->enc_ipv6.dst,
1434 TCA_FLOWER_KEY_ENC_IPV6_DST_MASK,
1435 sizeof(key->enc_ipv6.dst))))
1436 goto nla_put_failure;
1437
1438 if (fl_dump_key_val(skb, &key->enc_key_id, TCA_FLOWER_KEY_ENC_KEY_ID,
1439 &mask->enc_key_id, TCA_FLOWER_UNSPEC,
1440 sizeof(key->enc_key_id)) ||
1441 fl_dump_key_val(skb, &key->enc_tp.src,
1442 TCA_FLOWER_KEY_ENC_UDP_SRC_PORT,
1443 &mask->enc_tp.src,
1444 TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK,
1445 sizeof(key->enc_tp.src)) ||
1446 fl_dump_key_val(skb, &key->enc_tp.dst,
1447 TCA_FLOWER_KEY_ENC_UDP_DST_PORT,
1448 &mask->enc_tp.dst,
1449 TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
1450 sizeof(key->enc_tp.dst)))
1451 goto nla_put_failure;
1452
1453 if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
1454 goto nla_put_failure;
1455
1456 if (f->flags && nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags))
1457 goto nla_put_failure;
1458
1459 if (tcf_exts_dump(skb, &f->exts))
1460 goto nla_put_failure;
1461
1462 nla_nest_end(skb, nest);
1463
1464 if (tcf_exts_dump_stats(skb, &f->exts) < 0)
1465 goto nla_put_failure;
1466
1467 return skb->len;
1468
1469 nla_put_failure:
1470 nla_nest_cancel(skb, nest);
1471 return -1;
1472 }
1473
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next 0/9] nfp: flower updates and netconsole
From: David Miller @ 2018-06-30 12:32 UTC (permalink / raw)
To: jakub.kicinski; +Cc: oss-drivers, netdev
In-Reply-To: <20180630000442.27353-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 29 Jun 2018 17:04:33 -0700
> This set contains assorted updates to driver base and flower.
> First patch is a follow up to a fix to calculating counters which
> went into net. For ethtool counters we should also make sure
> they are visible even after ring reconfiguration. Next patch
> is a safety measure in case we are running on a machine with a
> broken BIOS we should fail the probe when risk of incorrect
> operation is too high. The next two patches add netpoll support
> and make use of napi_consume_skb(). Last we populate bus info
> on all representors.
>
> Pieter adds support for offload of the checksum action in flower.
>
> John follows up to another fix he's done in net, we set TTL
> values on tunnels to stack's default, now Johns does a full
> route lookup to get a more precise information, he populates
> ToS field as well. Last but not least he follows up on Jiri's
> request to enable LAG offload in case the team driver is used
> and then hash function is unknown.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net] hv_netvsc: split sub-channel setup into async and sync
From: David Miller @ 2018-06-30 12:29 UTC (permalink / raw)
To: haiyangz, haiyangz; +Cc: olaf, sthemmin, netdev, linux-kernel, devel, vkuznets
In-Reply-To: <20180629210716.6798-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Fri, 29 Jun 2018 14:07:16 -0700
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> When doing device hotplug the sub channel must be async to avoid
> deadlock issues because device is discovered in softirq context.
>
> When doing changes to MTU and number of channels, the setup
> must be synchronous to avoid races such as when MTU and device
> settings are done in a single ip command.
>
> Reported-by: Thomas Walker <Thomas.Walker@twosigma.com>
> Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
> Fixes: 732e49850c5e ("netvsc: fix race on sub channel creation")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [Patch net] net: use dev_change_tx_queue_len() for SIOCSIFTXQLEN
From: David Miller @ 2018-06-30 12:27 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, edumazet
In-Reply-To: <20180629204248.28919-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 29 Jun 2018 13:42:48 -0700
> As noticed by Eric, we need to switch to the helper
> dev_change_tx_queue_len() for SIOCSIFTXQLEN call path too,
> otheriwse still miss dev_qdisc_change_tx_queue_len().
>
> Fixes: 6a643ddb5624 ("net: introduce helper dev_change_tx_queue_len()")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] strparser: Call skb_unclone conditionally
From: David Miller @ 2018-06-30 12:25 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, tom, borisp, ebiggers, davejwatson, doronrk
In-Reply-To: <20180629191555.19874-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Sat, 30 Jun 2018 00:45:55 +0530
> Calling skb_unclone() is expensive as it triggers a memcpy operation.
> Instead of calling skb_unclone() unconditionally, call it only when skb
> has a shared frag_list. This improves tls rx throughout significantly.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> Suggested-by: Boris Pismenny <borisp@mellanox.com>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox