* Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Steffen Klassert @ 2017-10-25 4:25 UTC (permalink / raw)
To: Jonathan Basseri 😶
Cc: netdev, Jakub Sitnicki, David Miller, Lorenzo Colitti,
Eric Dumazet
In-Reply-To: <CABcqWgLEQ6j0FDPOQmMpQugPonBoUfKB2sfnP8+bJtYdNS8Viw@mail.gmail.com>
On Tue, Oct 24, 2017 at 09:58:48AM -0700, Jonathan Basseri 😶 wrote:
> On Tue, Oct 24, 2017 at 12:04 AM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > On Mon, Oct 23, 2017 at 06:18:55PM -0700, Jonathan Basseri wrote:
> > > If a socket has a valid dst cache, then xfrm_lookup_route will get
> > > skipped. However, the cache is not invalidated when applying policy to a
> > > socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
> > > sometimes ignored on those sockets. (Note: This was broken for IPv4 and
> > > IPv6 at different times.)
> > >
> > > This can be demonstrated like so,
> > > 1. Create UDP socket.
> > > 2. connect() the socket.
> > > 3. Apply an outbound XFRM policy to the socket.
> > > 4. send() data on the socket.
> > >
> > > Packets will continue to be sent in the clear instead of matching an
> > > xfrm or returning a no-match error (EAGAIN). This affects calls to
> > > send() and not sendto().
> > >
> > > Invalidating the sk_dst_cache is necessary to correctly apply xfrm
> > > policies. Since we do this in xfrm_user_policy(), the sk_lock was
> > > already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
> > > and we may call __sk_dst_reset().
> > >
> > > Performance impact should be negligible, since this code is only called
> > > when changing xfrm policy, and only affects the socket in question.
> > >
> > > Note: Creating normal XFRM policies should have a similar effect on
> > > sk_dst_cache entries that match the policy, but that is not fixed in
> > > this patch.
> >
> > I think we don't have this problem with 'normal' policies. When
> > inserting such a policy, we bump the IPv4/IPv6 genid. This should
> > invalidate all cached dst entries, no?
> >
> That sounds reasonable to me. I had not confirmed the behavior for
> normal policies, so I was trying to point out that this fix is only
> for socket policies. Should I modify the commit message?
Yes, please do so. This comment may lead people to the wrong direction.
Thanks!
^ permalink raw reply
* [PATCH net-next 0/4] net: qualcomm: rmnet: Add 64 bit stats and GRO
From: Subash Abhinov Kasiviswanathan @ 2017-10-25 4:55 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
This series adds support for 64 bit per cpu stats and GRO
Patches 1-2 are cleanups of return code and a redundant condition
Patch 3 adds support for 64 bit per cpu stats
Patch 4 adds support for GRO using GRO cells
Subash Abhinov Kasiviswanathan (4):
net: qualcomm: rmnet: Fix the return value of rmnet_rx_handler()
net: qualcomm: rmnet: Always assign rmnet dev in deaggregation path
net: qualcomm: rmnet: Add support for 64 bit stats
net: qualcomm: rmnet: Add support for GRO
drivers/net/ethernet/qualcomm/rmnet/Kconfig | 1 +
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 6 +-
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 16 ++++
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 38 ++++------
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 3 +-
.../ethernet/qualcomm/rmnet/rmnet_map_command.c | 4 +-
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 85 ++++++++++++++++++++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 2 +-
8 files changed, 118 insertions(+), 37 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next 1/4] net: qualcomm: rmnet: Fix the return value of rmnet_rx_handler()
From: Subash Abhinov Kasiviswanathan @ 2017-10-25 4:55 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1508907329-1077-1-git-send-email-subashab@codeaurora.org>
Since packet is always consumed by rmnet_rx_handler(), we always
return RX_HANDLER_CONSUMED. There is no need to pass on this
value through multiple functions.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 31 +++++++++-------------
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 3 +--
.../ethernet/qualcomm/rmnet/rmnet_map_command.c | 4 +--
3 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index df3d2d1..5dd186d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -43,7 +43,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
/* Generic handler */
-static rx_handler_result_t
+static void
rmnet_deliver_skb(struct sk_buff *skb)
{
skb_reset_transport_header(skb);
@@ -53,12 +53,11 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
skb->pkt_type = PACKET_HOST;
skb_set_mac_header(skb, 0);
netif_receive_skb(skb);
- return RX_HANDLER_CONSUMED;
}
/* MAP handler */
-static rx_handler_result_t
+static void
__rmnet_map_ingress_handler(struct sk_buff *skb,
struct rmnet_port *port)
{
@@ -91,31 +90,27 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
skb_pull(skb, sizeof(struct rmnet_map_header));
skb_trim(skb, len);
rmnet_set_skb_proto(skb);
- return rmnet_deliver_skb(skb);
+ rmnet_deliver_skb(skb);
+ return;
free_skb:
kfree_skb(skb);
- return RX_HANDLER_CONSUMED;
}
-static rx_handler_result_t
+static void
rmnet_map_ingress_handler(struct sk_buff *skb,
struct rmnet_port *port)
{
struct sk_buff *skbn;
- int rc;
if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
__rmnet_map_ingress_handler(skbn, port);
consume_skb(skb);
- rc = RX_HANDLER_CONSUMED;
} else {
- rc = __rmnet_map_ingress_handler(skb, port);
+ __rmnet_map_ingress_handler(skb, port);
}
-
- return rc;
}
static int rmnet_map_egress_handler(struct sk_buff *skb,
@@ -149,15 +144,13 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
return RMNET_MAP_SUCCESS;
}
-static rx_handler_result_t
+static void
rmnet_bridge_handler(struct sk_buff *skb, struct net_device *bridge_dev)
{
if (bridge_dev) {
skb->dev = bridge_dev;
dev_queue_xmit(skb);
}
-
- return RX_HANDLER_CONSUMED;
}
/* Ingress / Egress Entry Points */
@@ -168,13 +161,12 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
*/
rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
{
- int rc = RX_HANDLER_CONSUMED;
struct sk_buff *skb = *pskb;
struct rmnet_port *port;
struct net_device *dev;
if (!skb)
- return RX_HANDLER_CONSUMED;
+ goto done;
dev = skb->dev;
port = rmnet_get_port(dev);
@@ -182,14 +174,15 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
switch (port->rmnet_mode) {
case RMNET_EPMODE_VND:
if (port->ingress_data_format & RMNET_INGRESS_FORMAT_MAP)
- rc = rmnet_map_ingress_handler(skb, port);
+ rmnet_map_ingress_handler(skb, port);
break;
case RMNET_EPMODE_BRIDGE:
- rc = rmnet_bridge_handler(skb, port->bridge_ep);
+ rmnet_bridge_handler(skb, port->bridge_ep);
break;
}
- return rc;
+done:
+ return RX_HANDLER_CONSUMED;
}
/* Modifies packet as per logical endpoint configuration and egress data format
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index ce2302c..3af3fe7 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -80,7 +80,6 @@ struct rmnet_map_header {
struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb);
struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
int hdrlen, int pad);
-rx_handler_result_t rmnet_map_command(struct sk_buff *skb,
- struct rmnet_port *port);
+void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
#endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
index 74d362f..51e6049 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
@@ -76,8 +76,7 @@ static void rmnet_map_send_ack(struct sk_buff *skb,
/* Process MAP command frame and send N/ACK message as appropriate. Message cmd
* name is decoded here and appropriate handler is called.
*/
-rx_handler_result_t rmnet_map_command(struct sk_buff *skb,
- struct rmnet_port *port)
+void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
{
struct rmnet_map_control_command *cmd;
unsigned char command_name;
@@ -102,5 +101,4 @@ rx_handler_result_t rmnet_map_command(struct sk_buff *skb,
}
if (rc == RMNET_MAP_COMMAND_ACK)
rmnet_map_send_ack(skb, rc);
- return RX_HANDLER_CONSUMED;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: qualcomm: rmnet: Always assign rmnet dev in deaggregation path
From: Subash Abhinov Kasiviswanathan @ 2017-10-25 4:55 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1508907329-1077-1-git-send-email-subashab@codeaurora.org>
The rmnet device needs to assigned for all packets in the
deaggregation path based on the mux id, so the check is not needed.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 5dd186d..1ea9783 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -83,8 +83,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
if (!ep)
goto free_skb;
- if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEMUXING)
- skb->dev = ep->egress_dev;
+ skb->dev = ep->egress_dev;
/* Subtract MAP header */
skb_pull(skb, sizeof(struct rmnet_map_header));
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 3/4] net: qualcomm: rmnet: Add support for 64 bit stats
From: Subash Abhinov Kasiviswanathan @ 2017-10-25 4:55 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1508907329-1077-1-git-send-email-subashab@codeaurora.org>
Implement 64 bit per cpu stats.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 14 ++++
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 76 ++++++++++++++++++++--
2 files changed, 85 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 60115e6..9586703d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -41,9 +41,23 @@ struct rmnet_port {
extern struct rtnl_link_ops rmnet_link_ops;
+struct rmnet_vnd_stats {
+ u64 rx_pkts;
+ u64 rx_bytes;
+ u64 tx_pkts;
+ u64 tx_bytes;
+ u32 tx_drops;
+};
+
+struct rmnet_pcpu_stats {
+ struct rmnet_vnd_stats stats;
+ struct u64_stats_sync syncp;
+};
+
struct rmnet_priv {
u8 mux_id;
struct net_device *real_dev;
+ struct rmnet_pcpu_stats __percpu *pcpu_stats;
};
struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 12bd0bb..b0befa1 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -27,14 +27,28 @@
void rmnet_vnd_rx_fixup(struct sk_buff *skb, struct net_device *dev)
{
- dev->stats.rx_packets++;
- dev->stats.rx_bytes += skb->len;
+ struct rmnet_priv *priv = netdev_priv(dev);
+ struct rmnet_pcpu_stats *pcpu_ptr;
+
+ pcpu_ptr = this_cpu_ptr(priv->pcpu_stats);
+
+ u64_stats_update_begin(&pcpu_ptr->syncp);
+ pcpu_ptr->stats.rx_pkts++;
+ pcpu_ptr->stats.rx_bytes += skb->len;
+ u64_stats_update_end(&pcpu_ptr->syncp);
}
void rmnet_vnd_tx_fixup(struct sk_buff *skb, struct net_device *dev)
{
- dev->stats.tx_packets++;
- dev->stats.tx_bytes += skb->len;
+ struct rmnet_priv *priv = netdev_priv(dev);
+ struct rmnet_pcpu_stats *pcpu_ptr;
+
+ pcpu_ptr = this_cpu_ptr(priv->pcpu_stats);
+
+ u64_stats_update_begin(&pcpu_ptr->syncp);
+ pcpu_ptr->stats.tx_pkts++;
+ pcpu_ptr->stats.tx_bytes += skb->len;
+ u64_stats_update_end(&pcpu_ptr->syncp);
}
/* Network Device Operations */
@@ -48,7 +62,7 @@ static netdev_tx_t rmnet_vnd_start_xmit(struct sk_buff *skb,
if (priv->real_dev) {
rmnet_egress_handler(skb);
} else {
- dev->stats.tx_dropped++;
+ this_cpu_inc(priv->pcpu_stats->stats.tx_drops);
kfree_skb(skb);
}
return NETDEV_TX_OK;
@@ -70,12 +84,64 @@ static int rmnet_vnd_get_iflink(const struct net_device *dev)
return priv->real_dev->ifindex;
}
+static int rmnet_vnd_init(struct net_device *dev)
+{
+ struct rmnet_priv *priv = netdev_priv(dev);
+
+ priv->pcpu_stats = alloc_percpu(struct rmnet_pcpu_stats);
+ if (!priv->pcpu_stats)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void rmnet_vnd_uninit(struct net_device *dev)
+{
+ struct rmnet_priv *priv = netdev_priv(dev);
+
+ free_percpu(priv->pcpu_stats);
+}
+
+static void rmnet_get_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *s)
+{
+ struct rmnet_priv *priv = netdev_priv(dev);
+ struct rmnet_vnd_stats total_stats;
+ struct rmnet_pcpu_stats *pcpu_ptr;
+ unsigned int cpu, start;
+
+ memset(&total_stats, 0, sizeof(struct rmnet_vnd_stats));
+
+ for_each_possible_cpu(cpu) {
+ pcpu_ptr = this_cpu_ptr(priv->pcpu_stats);
+
+ do {
+ start = u64_stats_fetch_begin_irq(&pcpu_ptr->syncp);
+ total_stats.rx_pkts += pcpu_ptr->stats.rx_pkts;
+ total_stats.rx_bytes += pcpu_ptr->stats.rx_bytes;
+ total_stats.tx_pkts += pcpu_ptr->stats.tx_pkts;
+ total_stats.tx_bytes += pcpu_ptr->stats.tx_bytes;
+ } while (u64_stats_fetch_retry_irq(&pcpu_ptr->syncp, start));
+
+ total_stats.tx_drops += pcpu_ptr->stats.tx_drops;
+ }
+
+ s->rx_packets = total_stats.rx_pkts;
+ s->rx_bytes = total_stats.rx_bytes;
+ s->tx_packets = total_stats.tx_pkts;
+ s->tx_bytes = total_stats.tx_bytes;
+ s->tx_dropped = total_stats.tx_drops;
+}
+
static const struct net_device_ops rmnet_vnd_ops = {
.ndo_start_xmit = rmnet_vnd_start_xmit,
.ndo_change_mtu = rmnet_vnd_change_mtu,
.ndo_get_iflink = rmnet_vnd_get_iflink,
.ndo_add_slave = rmnet_add_bridge,
.ndo_del_slave = rmnet_del_bridge,
+ .ndo_init = rmnet_vnd_init,
+ .ndo_uninit = rmnet_vnd_uninit,
+ .ndo_get_stats64 = rmnet_get_stats64,
};
/* Called by kernel whenever a new rmnet<n> device is created. Sets MTU,
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: qualcomm: rmnet: Add support for GRO
From: Subash Abhinov Kasiviswanathan @ 2017-10-25 4:55 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1508907329-1077-1-git-send-email-subashab@codeaurora.org>
Add gro_cells so that rmnet devices can call gro_cells_receive
instead of netif_receive_skb.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/Kconfig | 1 +
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 6 +++---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 2 ++
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 4 +++-
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 9 ++++++++-
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 2 +-
6 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/Kconfig b/drivers/net/ethernet/qualcomm/rmnet/Kconfig
index 6e2587a..9bb06d2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/Kconfig
+++ b/drivers/net/ethernet/qualcomm/rmnet/Kconfig
@@ -5,6 +5,7 @@
menuconfig RMNET
tristate "RmNet MAP driver"
default n
+ select GRO_CELLS
---help---
If you select this, you will enable the RMNET module which is used
for handling data in the multiplexing and aggregation protocol (MAP)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 71bee1a..5ce4287 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -191,7 +191,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
return 0;
err2:
- rmnet_vnd_dellink(mux_id, port, ep);
+ rmnet_vnd_dellink(mux_id, port, ep, dev);
err1:
rmnet_unregister_real_device(real_dev, port);
err0:
@@ -221,7 +221,7 @@ static void rmnet_dellink(struct net_device *dev, struct list_head *head)
if (ep) {
hlist_del_init_rcu(&ep->hlnode);
rmnet_unregister_bridge(dev, port);
- rmnet_vnd_dellink(mux_id, port, ep);
+ rmnet_vnd_dellink(mux_id, port, ep, dev);
kfree(ep);
}
rmnet_unregister_real_device(real_dev, port);
@@ -239,7 +239,7 @@ static int rmnet_dev_walk_unreg(struct net_device *rmnet_dev, void *data)
ep = rmnet_get_endpoint(d->port, mux_id);
if (ep) {
hlist_del_init_rcu(&ep->hlnode);
- rmnet_vnd_dellink(mux_id, d->port, ep);
+ rmnet_vnd_dellink(mux_id, d->port, ep, rmnet_dev);
kfree(ep);
}
netdev_upper_dev_unlink(rmnet_dev, d->real_dev);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 9586703d..c19259e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -14,6 +14,7 @@
*/
#include <linux/skbuff.h>
+#include <net/gro_cells.h>
#ifndef _RMNET_CONFIG_H_
#define _RMNET_CONFIG_H_
@@ -58,6 +59,7 @@ struct rmnet_priv {
u8 mux_id;
struct net_device *real_dev;
struct rmnet_pcpu_stats __percpu *pcpu_stats;
+ struct gro_cells gro_cells;
};
struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 1ea9783..29842cc 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -46,13 +46,15 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
static void
rmnet_deliver_skb(struct sk_buff *skb)
{
+ struct rmnet_priv *priv = netdev_priv(skb->dev);
+
skb_reset_transport_header(skb);
skb_reset_network_header(skb);
rmnet_vnd_rx_fixup(skb, skb->dev);
skb->pkt_type = PACKET_HOST;
skb_set_mac_header(skb, 0);
- netif_receive_skb(skb);
+ gro_cells_receive(&priv->gro_cells, skb);
}
/* MAP handler */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index b0befa1..45a97eb 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -149,6 +149,8 @@ static void rmnet_get_stats64(struct net_device *dev,
*/
void rmnet_vnd_setup(struct net_device *rmnet_dev)
{
+ struct rmnet_priv *priv = netdev_priv(rmnet_dev);
+
rmnet_dev->netdev_ops = &rmnet_vnd_ops;
rmnet_dev->mtu = RMNET_DFLT_PACKET_SIZE;
rmnet_dev->needed_headroom = RMNET_NEEDED_HEADROOM;
@@ -162,6 +164,8 @@ void rmnet_vnd_setup(struct net_device *rmnet_dev)
rmnet_dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
rmnet_dev->needs_free_netdev = true;
+
+ gro_cells_init(&priv->gro_cells, rmnet_dev);
}
/* Exposed API */
@@ -196,11 +200,14 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
}
int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
- struct rmnet_endpoint *ep)
+ struct rmnet_endpoint *ep, struct net_device *rmnet_dev)
{
+ struct rmnet_priv *priv = netdev_priv(rmnet_dev);
+
if (id >= RMNET_MAX_LOGICAL_EP || !ep->egress_dev)
return -EINVAL;
+ gro_cells_destroy(&priv->gro_cells);
ep->egress_dev = NULL;
port->nr_rmnet_devs--;
return 0;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
index 71e4c32..18cd5f6c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
@@ -22,7 +22,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
struct net_device *real_dev,
struct rmnet_endpoint *ep);
int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
- struct rmnet_endpoint *ep);
+ struct rmnet_endpoint *ep, struct net_device *rmnet_dev);
void rmnet_vnd_rx_fixup(struct sk_buff *skb, struct net_device *dev);
void rmnet_vnd_tx_fixup(struct sk_buff *skb, struct net_device *dev);
u8 rmnet_vnd_get_mux(struct net_device *rmnet_dev);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] drivers/wireless: iwlwifi/mvm: Convert timers to use timer_setup()
From: Luca Coelho @ 2017-10-25 5:33 UTC (permalink / raw)
To: Kees Cook, Kalle Valo
Cc: Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
Sara Sharon, linux-wireless, netdev, linux-kernel
In-Reply-To: <20171024092937.GA47262@beast>
On Tue, 2017-10-24 at 02:29 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to
> all timer callbacks, switch to using the new timer_setup() and
> from_timer()
> to pass the timer pointer explicitly.
>
> The RCU lifetime on baid_data is unclear, so this adds a direct copy
> of the
> rcu_ptr passed to the original callback. It may be possible to
> improve this
> to just use baid_data->mvm->baid_map[baid_data->baid] instead.
>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Cc: Intel Linux Wireless <linuxwifi@intel.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Sara Sharon <sara.sharon@intel.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
Thanks, Kees. I'm taking this for review on our internal tree. If all
our checks pass, I'll apply it and it will reach the mainline following
our usual upstreaming process.
--
Cheers,
Luca.
^ permalink raw reply
* RE: [PATCH net-next v3 01/10] devlink: Add permanent config parameter get/set operations
From: Yuval Mintz @ 2017-10-25 5:41 UTC (permalink / raw)
To: Steve Lin
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com,
netdev@vger.kernel.org
In-Reply-To: <CA+Jmh7FGWf=WVsVZzsLHvZpeo81iG7gzerF94T8dtn5SpUj+VQ@mail.gmail.com>
> On Tue, Oct 24, 2017 at 5:22 PM, Yuval Mintz <yuvalm@mellanox.com>
> wrote:
> >> Add support for permanent config parameter get/set commands. Used
> >> for persistent device configuration parameters.
> >>
> > ...
> >> + int (*perm_config_get)(struct devlink *devlink,
> >> + enum devlink_perm_config_param param, u8
> >> type,
> >> + void *value);
> >> + int (*perm_config_set)(struct devlink *devlink,
> >> + enum devlink_perm_config_param param, u8
> >> type,
> >> + void *value, u8 *restart_reqd);
> >> };
> >> +static int devlink_nl_single_param_get(struct sk_buff *msg,
> >> + struct devlink *devlink,
> >> + u32 param, u8 type)
> >> +{
> >> + const struct devlink_ops *ops = devlink->ops;
> >> + struct nlattr *param_attr;
> >> + void *value;
> >> + u32 val;
> >> + int err;
> >> +
> >> + /* Allocate buffer for parameter value */
> >> + switch (type) {
> >> + case NLA_U8:
> >> + value = kmalloc(sizeof(u8), GFP_KERNEL);
> >> + break;
> >> + case NLA_U16:
> >> + value = kmalloc(sizeof(u16), GFP_KERNEL);
> >> + break;
> >> + case NLA_U32:
> >> + value = kmalloc(sizeof(u32), GFP_KERNEL);
> >> + break;
> >> + default:
> >> + return -EINVAL; /* Unsupported Type */
> >> + }
> >> +
> >> + if (!value)
> >> + return -ENOMEM;
> >> +
> >> + err = ops->perm_config_get(devlink, param, type, value);
> >> + if (err)
> >> + return err;
> >
> > I suspect this logic might be risky - its dependent on the driver to cast the
> > 'value' into the proper type or else, E.g., the following switch might break
> > for BE platforms.
> > Is there any reason to have the devlink <-> driver API be based on void*
> > and not on some typed data [of sufficient size]?
> > ...
> >> + switch (type) {
> >> + case NLA_U8:
> >> + val = *((u8 *)value);
> >> + if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
> >> val))
> >> + goto nest_err;
> >> + break;
> >> + case NLA_U16:
> >> + val = *((u16 *)value);
> >> + if (nla_put_u16(msg,
> >> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
> >> + goto nest_err;
> >> + break;
> >> + case NLA_U32:
> >> + val = *((u32 *)value);
> >> + if (nla_put_u32(msg,
> >> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
> >> + goto nest_err;
> >> + break;
> >> + }
>
> Why might this break on a BE system? It's not as though driver will
> be compiled LE and kernel BE or vice versa - as long as driver and
> kernel are same endian-ness, I would think it should be okay?
It depends on the driver implementation to cast your pointer to the right type.
E.g., driver needs to fill in a u8 data in *value for a given parameter.
If the driver casted the pointer to (u8*) everything is fine. But if he casted
it to (u32*) [naïve implementation that doesn't care about the type]
and filled it, then on a LE machine value[0] would contain the data while on
BE value[3] would contain it.
>
> In general, the issue is that the parameter could be any of the
> netlink types (per Jiri's suggestion to the previous version of this
> patch). So, we allocate some space, tell the driver the type we're
> expecting (the type argument to the perm_config_get() function), and
> yes, we rely on the driver to write something of the type we request
> to the pointer we provide. Are you suggesting defining a union of
> U8/U16/U32, and passing a pointer to that for the driver to fill in?
Problem is that the driver-side could always use the biggest data-type
as long as it's working on a LE machine, but that approach would break
if same driver would be tried on a BE machine.
And the developer would have no way of knowing other than via code-review.
> The issue is that whatever the types we support now, we want future
> parameters to be able to be of arbitrary types. Defining the
> interface to use the void pointer means that some future parameter can
> be of some other type, without having to update all the drivers using
> this API...
>
> Or did I misunderstand your suggestion?
^ permalink raw reply
* RE: [PATCH net-next v3 02/10] devlink: Adding SR-IOV enablement perm config param
From: Yuval Mintz @ 2017-10-25 5:41 UTC (permalink / raw)
To: Steve Lin, netdev@vger.kernel.org
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <1508875962-39233-3-git-send-email-steven.lin1@broadcom.com>
> Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
> parameter. Value is permanent, so becomes the new default
> value for this device.
>
> 0 = Disable SR-IOV
> 1 = Enable SR-IOV
Does this imposes a requirement on the PCIe specifics, E.g., that the device
should no longer expose the SRIOV PCie capability?
Or does any implementation that would prevent a user from activating
SR-IOV is sufficient?
^ permalink raw reply
* RE: [PATCH net-next v3 03/10] devlink: Adding num VFs per PF permanent config param
From: Yuval Mintz @ 2017-10-25 5:41 UTC (permalink / raw)
To: Steve Lin, netdev@vger.kernel.org
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <1508875962-39233-4-git-send-email-steven.lin1@broadcom.com>
> Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
> parameter. Value is permanent, so becomes the new default
> value for this device.
>
> The value sets the number of VFs per PF in SR-IOV mode.
Assuming it's meant to directly control the PCIe capability value
I think you should mention it explicitly in the commit message.
^ permalink raw reply
* RE: [PATCH net-next v3 06/10] bnxt: Add devlink support for config get/set
From: Yuval Mintz @ 2017-10-25 5:55 UTC (permalink / raw)
To: Steve Lin, netdev@vger.kernel.org
Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <1508875962-39233-7-git-send-email-steven.lin1@broadcom.com>
> +static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
> + void *buf, int size)
> +{
> + struct hwrm_nvm_get_variable_input req = {0};
> + dma_addr_t dest_data_dma_addr;
> + void *dest_data_addr = NULL;
> + int bytesize;
> + int rc;
> +
> + bytesize = (size + 7) / BITS_PER_BYTE;
roundup?
..
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
> + const void *buf, int size)
> +{
> + struct hwrm_nvm_set_variable_input req = {0};
> + dma_addr_t src_data_dma_addr;
> + void *src_data_addr = NULL;
> + int bytesize;
> + int rc;
> +
> + bytesize = (size + 7) / BITS_PER_BYTE;
Likewise
> +
> + src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
> + &src_data_dma_addr,
> GFP_KERNEL);
> + if (!src_data_addr) {
> + netdev_err(bp->dev, "dma_alloc_coherent failure\n");
Won't you see an oom? Why do you need the print?
> +static int bnxt_dl_perm_config_set(struct devlink *devlink,
> + enum devlink_perm_config_param param,
> + u8 type, void *value, u8 *restart_reqd)
> +{
> + struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
> + struct bnxt_drv_cfgparam *entry;
> + int idx = 0;
> + int ret = 0;
> + u32 bytesize;
> + u32 val32;
> + u16 val16;
> + u8 val8;
> + int i;
> +
> + *restart_reqd = 0;
> +
> + /* Find parameter in table */
> + for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
> + if (param == bnxt_drv_cfgparam_list[i].param) {
> + entry = &bnxt_drv_cfgparam_list[i];
> + break;
> + }
> + }
> +
> + /* Not found */
> + if (i == BNXT_NUM_DRV_CFGPARAM)
> + return -EINVAL;
> +
Looks cleaner to check whether entry is set instead
...
> + bytesize = (entry->bitlength + 7) / BITS_PER_BYTE;
Roundup?
...
> + if (bytesize == 1) {
> + val8 = val32;
Don't you need explicit castings for these kind of assignments
to prevent warnings?
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val8,
> + entry->bitlength);
> + } else if (bytesize == 2) {
> + val16 = val32;
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val16,
> + entry->bitlength);
> + } else {
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val32,
> + entry->bitlength);
> + }
> + }
> +
> + /* Restart required for all nvm parameter writes */
> + *restart_reqd = 1;
> +
> + return ret;
> +}
> +
> +static int bnxt_dl_perm_config_get(struct devlink *devlink,
> + enum devlink_perm_config_param param,
> + u8 type, void *value)
> +{
Same comments as for the setter
...
> - if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
> - return 0;
> -
> - if (bp->hwrm_spec_code < 0x10800) {
> + if ((!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV)) ||
> + bp->hwrm_spec_code < 0x10800) {
> + /* eswitch switchdev mode not supported */
> + bnxt_dl_ops.eswitch_mode_set = NULL;
> + bnxt_dl_ops.eswitch_mode_get = NULL;
Why would you need to tie this interface to the presence of SRIOV in PCIe?
Also, Assuming the ability to disable sriov in #2 would cause this capability
not to be exposed after reboot, isn't this a one-way ticket?
^ permalink raw reply
* Re: [PATCH net-next 00/10] korina cleanups/optimizations
From: Roman Yeryomin @ 2017-10-25 6:19 UTC (permalink / raw)
To: David Miller; +Cc: f.fainelli, netdev
In-Reply-To: <cc15f52b1f8d909b13b178b7abadb4dd@advem.lv>
On 2017-10-22 23:57, Roman Yeryomin wrote:
> On 2017-10-16 00:05, David Miller wrote:
>> From: Roman Yeryomin <roman@advem.lv>
>> Date: Sun, 15 Oct 2017 19:46:02 +0300
>>
>>> On 2017-10-15 19:38, Florian Fainelli wrote:
>>>> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv>
>>>> wrote:
>>>>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>>>>> in local tx usecase (tested with iperf v3.2).
>>>> Could you avoid empty commit messages and write a paragraph or two
>>>> for
>>>> each commit that explains what and why are you changing? The changes
>>>> look fine but they lack any explanation.
>>>
>>> I thought that short descriptions are already self explanatory and
>>> just didn't know what to write more.
>>
>> "Optimize TX handlers."
>>
>> In what way? Why? How are things improved? Is it measurable?
>> etc.
>
> OK, got the idea.
> However I think I would need some help with measuring performance
> difference reliably.
> On this CPU iperf3 tx takes most of the time (like 80-90%), thus even
> well optimized changes will be hard to see with iperf3 alone.
> I've tried using pktgen module. Although it shows much better numbers
> than iperf3 (~95Mbps vs. 40), results don't look like very
> stable/reliable, pps may differ by 10-15% easily between different
> runs.
> perf. I have limited experience with it but if I understand correctly,
> this CPU doesn't support neither cycles nor instructions counters. So
> not sure if perf would be useful here.
>
> Performance counter stats for 'system wide':
>
> 10387.717082 cpu-clock (msec) # 1.000 CPUs
> utilized
> 2941 context-switches # 0.283 K/sec
> 0 cpu-migrations # 0.000 K/sec
> 60 page-faults # 0.006 K/sec
> <not supported> cycles
> <not supported> instructions
> <not supported> branches
> <not supported> branch-misses
>
> 10.388087500 seconds time elapsed
>
>
> What are the suggestions?
Any ideas?
Or I can just comment on the patch(es) which gave apparent performance
improvement (as seen with iperf3) and others mark as cleanup.
Regards,
Roman
^ permalink raw reply
* Re: [iproute PATCH] ss: add detail explains of -m, -o, -e and -i options in ss man page
From: Stephen Hemminger @ 2017-10-25 6:37 UTC (permalink / raw)
To: peng yu; +Cc: netdev, phil
In-Reply-To: <CAG3TDc0XxtkA5TU_GDzdZv1ujuqf09e9kBYQm2p5QGbppt7efw@mail.gmail.com>
On Tue, 24 Oct 2017 17:13:30 -0700
peng yu <yupeng0921@gmail.com> wrote:
> +.SH Additional explain of -m, -o, -e and -i options
> +Some fields may have different meanings if the netowrk protocl is
> different. Below explain focus on tcp protocol.
Thank you for the patch.
Some formatting and style changes should be done before I accept it.
First, the SECTIONS in man pages are usually terse and all upper case
(back to Unix man page style). Why not just put the additional text as a second
paragraph on the part that explains the first option?
Also, please run all manual page submissions through a spell checker.
The ispell program is one option.
Please resubmit
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] bnxt: Add devlink support for config get/set
From: Michael Chan @ 2017-10-25 6:40 UTC (permalink / raw)
To: Yuval Mintz
Cc: Steve Lin, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <AM0PR0502MB3683860B5E15C71A00435499BF440@AM0PR0502MB3683.eurprd05.prod.outlook.com>
On Tue, Oct 24, 2017 at 10:55 PM, Yuval Mintz <yuvalm@mellanox.com> wrote:
> ...
>
>> - if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
>> - return 0;
>> -
>> - if (bp->hwrm_spec_code < 0x10800) {
>> + if ((!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV)) ||
>> + bp->hwrm_spec_code < 0x10800) {
>> + /* eswitch switchdev mode not supported */
>> + bnxt_dl_ops.eswitch_mode_set = NULL;
>> + bnxt_dl_ops.eswitch_mode_get = NULL;
>
> Why would you need to tie this interface to the presence of SRIOV in PCIe?
> Also, Assuming the ability to disable sriov in #2 would cause this capability
> not to be exposed after reboot, isn't this a one-way ticket?
>
No, he is modifying the existing devlink logic for the switchdev
eswitch mode. That logic controls the SRIOV eswitch mode and depends
on SRIOV.
But I agree the patch makes it look cumbersome. It will be better to
check for SRIOV and proper firmware support and then setup the
eswitch_mode devlink methods.
^ permalink raw reply
* Re: [PATCH] ethernet: cavium: octeon: Switch to using netdev_info().
From: kbuild test robot @ 2017-10-25 6:41 UTC (permalink / raw)
To: Steven J. Hill; +Cc: kbuild-all, netdev, Steven J. Hill, David Daney
In-Reply-To: <1508777005-20938-1-git-send-email-steven.hill@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]
Hi Steven,
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.14-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Steven-J-Hill/ethernet-cavium-octeon-Switch-to-using-netdev_info/20171024-071910
config: mips-cavium_octeon_defconfig (attached as .config)
compiler: mips64-linux-gnuabi64-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All warnings (new ones prefixed by >>):
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c: In function 'octeon_mgmt_adjust_link':
>> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c:929:5: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]
if (link_changed != 0)
^
vim +/else +929 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
896
897 static void octeon_mgmt_adjust_link(struct net_device *netdev)
898 {
899 struct octeon_mgmt *p = netdev_priv(netdev);
900 struct phy_device *phydev = netdev->phydev;
901 unsigned long flags;
902 int link_changed = 0;
903
904 if (!phydev)
905 return;
906
907 spin_lock_irqsave(&p->lock, flags);
908
909
910 if (!phydev->link && p->last_link)
911 link_changed = -1;
912
913 if (phydev->link &&
914 (p->last_duplex != phydev->duplex ||
915 p->last_link != phydev->link ||
916 p->last_speed != phydev->speed)) {
917 octeon_mgmt_disable_link(p);
918 link_changed = 1;
919 octeon_mgmt_update_link(p);
920 octeon_mgmt_enable_link(p);
921 }
922
923 p->last_link = phydev->link;
924 p->last_speed = phydev->speed;
925 p->last_duplex = phydev->duplex;
926
927 spin_unlock_irqrestore(&p->lock, flags);
928
> 929 if (link_changed != 0)
930 if (link_changed > 0)
931 netdev_info(netdev, "Link is up - %d/%s\n",
932 phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half");
933 else
934 netdev_info(netdev, "Link is down\n");
935 }
936
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16794 bytes --]
^ permalink raw reply
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: joeyli @ 2017-10-25 7:07 UTC (permalink / raw)
To: David Howells
Cc: Alexei Starovoitov, linux-security-module-u79uwXL29TY76Z2rM5mHXA,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jforbes-H+wXaHxf7aLQT0dZR+AlfA, Daniel Borkmann, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, Gary Lin
In-Reply-To: <21530.1508770380-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Mon, Oct 23, 2017 at 03:53:00PM +0100, David Howells wrote:
> jlee-IBi9RG/b67k@public.gmane.org wrote:
>
> > hm... patch 4 only prevents write_mem() but not read_mem().
> > Or I missed anything?
>
> Actually, yes, as it happens, patch 11 prevents you from even opening /dev/mem
> and /dev/kmem by locking down open of /dev/port. So I've moved this bit to
> patch 4, simplified and posted a new variant for patch 4.
>
Thank you for pointing out!
Joey Lee
^ permalink raw reply
* [PATCH] net/unix: don't show information about sockets from other namespaces
From: Andrei Vagin @ 2017-10-25 7:17 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Andrei Vagin
socket_diag shows information only about sockets from a namespace where
a diag socket lives.
But if we request information about one unix socket, the kernel don't
check that its netns is matched with a diag socket namespace, so any
user can get information about any unix socket in a system. This looks
like a bug.
Signed-off-by: Andrei Vagin <avagin@openvz.org>
---
net/unix/diag.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 4d96797..384c84e 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -257,6 +257,8 @@ static int unix_diag_get_exact(struct sk_buff *in_skb,
err = -ENOENT;
if (sk == NULL)
goto out_nosk;
+ if (!net_eq(sock_net(sk), net))
+ goto out;
err = sock_diag_check_cookie(sk, req->udiag_cookie);
if (err)
--
2.9.4
^ permalink raw reply related
* Re: [PATCH net-next 2/3] xfrm: Fix offload dev state addition to occur after insertion
From: Steffen Klassert @ 2017-10-25 7:22 UTC (permalink / raw)
To: avivh; +Cc: Herbert Xu, Boris Pismenny, Yossi Kuperman, Yevgeny Kliteynik,
netdev
In-Reply-To: <1508857831-55824-2-git-send-email-avivh@mellanox.com>
On Tue, Oct 24, 2017 at 06:10:30PM +0300, avivh@mellanox.com wrote:
> From: Aviv Heller <avivh@mellanox.com>
>
> Adding the state to the offload device prior to replay init in
> xfrm_state_construct() will result in NULL dereference if a matching
> ESP packet is received in between.
>
> Adding it after insertion also has the benefit of the driver not having
> to check whether a state with the same match criteria already exists,
> but forces us to use an atomic type for the offload_handle, to make
> certain a stack-read/driver-write race won't result in reading corrupt
> data.
No, this will add multiple atomic operations to the packet path,
even in the non offloaded case.
I think the problem is that we set XFRM_STATE_VALID to early.
This was not a problem before we had offloading because
it was not possible to lookup this state before we inserted
it into the SADB. Now that the driver holds a subset of states
too, we need to make sure the state is fully initialized
before we mark it as valid.
The patch below should do it, in combination with your patch 1/3.
Could you please test this?
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b997f13..96eb263 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -587,10 +587,6 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
if (attrs[XFRMA_OUTPUT_MARK])
x->props.output_mark = nla_get_u32(attrs[XFRMA_OUTPUT_MARK]);
- err = __xfrm_init_state(x, false, attrs[XFRMA_OFFLOAD_DEV]);
- if (err)
- goto error;
-
if (attrs[XFRMA_SEC_CTX]) {
err = security_xfrm_state_alloc(x,
nla_data(attrs[XFRMA_SEC_CTX]));
@@ -620,6 +616,10 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
/* override default values from above */
xfrm_update_ae_params(x, attrs, 0);
+ err = __xfrm_init_state(x, false, attrs[XFRMA_OFFLOAD_DEV]);
+ if (err)
+ goto error;
+
return x;
error:
^ permalink raw reply related
* Re: [RFC] net/unix_diag: Provide UDIAG_SHOW_VFS2 attribute to fetch complete inode number
From: Cyrill Gorcunov @ 2017-10-25 7:27 UTC (permalink / raw)
To: Andrei Vagin; +Cc: NETDEV, LKML, Andrey Vagin, David S. Miller, Pavel Emelyanov
In-Reply-To: <20171025002509.GA12438@outlook.office365.com>
On Tue, Oct 24, 2017 at 05:25:16PM -0700, Andrei Vagin wrote:
> On Wed, Oct 25, 2017 at 12:48:14AM +0300, Cyrill Gorcunov wrote:
> > Currently unix_diag_vfs structure reports unix socket inode
> > as u32 value which of course doesn't fit to ino_t type and
>
> BTW: As far as I understand, it is not a problem right now, because
> get_next_ino returns int. And I'm agree that it maybe a problem in a
> future and it is better to be ready.
>
> > the number may be trimmed. Lets rather deprecate old UDIAG_SHOW_VFS
> > interface and provide UDIAG_SHOW_VFS2 (with one field "__zero" reserved
> > which we could extend in future).
>
> There is one more place where we return ino as u32:
>
> static int sk_diag_dump_peer(struct sock *sk, struct sk_buff *nlskb)
> ....
> return nla_put_u32(nlskb, UNIX_DIAG_PEER, ino);
Managed to miss it, thanks!
> > +struct unix_diag_vfs2 {
> > + __u64 udiag_vfs_ino;
> > + __u32 udiag_vfs_dev;
> > + __u32 __zero; /* Reserve for future use */
>
> How can a user understand whether this field is used or not?
Checking out if it zero or not.
> Each netlink attribute has its size in a header. Any attribute can be
> extended, and users can understand which fields are filled by
> a size of an attribute.
Well, that's correct, but it implies that any extension has different
size. I though of extending this structure (if ever needed) the way
that same attribute may carry different structures equal in size and
setting up @__zero field with some bit would help. On the other side
it become more complex than needed, so now I think I should simply
drop __zero out.
Thanks for comments, Andrew!
Cyrill
^ permalink raw reply
* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Steffen Klassert @ 2017-10-25 7:28 UTC (permalink / raw)
To: David Miller; +Cc: herbert, garsilva, netdev, linux-kernel
In-Reply-To: <20171025.132222.1438740282932861791.davem@davemloft.net>
On Wed, Oct 25, 2017 at 01:22:22PM +0900, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Wed, 25 Oct 2017 12:05:41 +0800
>
> > On Tue, Oct 24, 2017 at 05:48:42PM +0900, David Miller wrote:
> >>
> >> This discussion has happened before.
> >>
> >> But I'll explain the conclusion here for your benefit.
> >>
> >> BUG_ON() is a statement and everything inside of it will
> >> always execute.
> >>
> >> BUG_ON() is always preferred because it allows arch
> >> specific code to pass the conditional result properly
> >> into inline asm and builtins for optimal code generation.
> >
> > This is a good point. However, while a little bit more verbose you
> > can still achieve the same assembly-level result by something like
> >
> > int err;
> >
> > err = <insert real code here>;
> > BUG_ON(err);
> >
> > Having real code in BUG_ON may pose problems to people reading the
> > code because some of us tend to ignore code in BUG_ON and similar
> > macros such as BUILD_BUG_ON.
>
> I agree that this makes the code easier to read and audit.
It seems that we have an agreement on the above version,
Gustavo can you please update your patches to this?
Thanks!
^ permalink raw reply
* [PATCH v2] tap: double-free in error path in tap_open()
From: Girish Moodalbail @ 2017-10-25 7:23 UTC (permalink / raw)
To: netdev, davem, jasowang
Double free of skb_array in tap module is causing kernel panic. When
tap_set_queue() fails we free skb_array right away by calling
skb_array_cleanup(). However, later on skb_array_cleanup() is called
again by tap_sock_destruct through sock_put(). This patch fixes that
issue.
Fixes: 362899b8725b35e3 (macvtap: switch to use skb array)
Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
---
v1 -> v2:
- took care of an another issue in failure path of skb_array_init
---
drivers/net/tap.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 21b71ae..98ee6cc 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -517,6 +517,10 @@ static int tap_open(struct inode *inode, struct file *file)
&tap_proto, 0);
if (!q)
goto err;
+ if (skb_array_init(&q->skb_array, tap->dev->tx_queue_len, GFP_KERNEL)) {
+ sk_free(&q->sk);
+ goto err;
+ }
RCU_INIT_POINTER(q->sock.wq, &q->wq);
init_waitqueue_head(&q->wq.wait);
@@ -540,22 +544,18 @@ static int tap_open(struct inode *inode, struct file *file)
if ((tap->dev->features & NETIF_F_HIGHDMA) && (tap->dev->features & NETIF_F_SG))
sock_set_flag(&q->sk, SOCK_ZEROCOPY);
- err = -ENOMEM;
- if (skb_array_init(&q->skb_array, tap->dev->tx_queue_len, GFP_KERNEL))
- goto err_array;
-
err = tap_set_queue(tap, file, q);
- if (err)
- goto err_queue;
+ if (err) {
+ /* tap_sock_destruct() will take care of freeing skb_array */
+ goto err_put;
+ }
dev_put(tap->dev);
rtnl_unlock();
return err;
-err_queue:
- skb_array_cleanup(&q->skb_array);
-err_array:
+err_put:
sock_put(&q->sk);
err:
if (tap)
--
1.8.3.1
^ permalink raw reply related
* RE: [Patch net-next] tipc: fix a dangling pointer
From: Jon Maloy @ 2017-10-25 7:58 UTC (permalink / raw)
To: Cong Wang, netdev@vger.kernel.org; +Cc: Ying Xue
In-Reply-To: <20171024224449.8197-1-xiyou.wangcong@gmail.com>
Acked. Thank you.
///jon
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Wednesday, October 25, 2017 00:45
> To: netdev@vger.kernel.org
> Cc: Cong Wang <xiyou.wangcong@gmail.com>; Jon Maloy
> <jon.maloy@ericsson.com>; Ying Xue <ying.xue@windriver.com>
> Subject: [Patch net-next] tipc: fix a dangling pointer
>
> tsk->group is set to grp earlier, but we forget to unset it
> after grp is freed.
>
> Fixes: 75da2163dbb6 ("tipc: introduce communication groups")
> Reported-by: syzkaller bot
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/tipc/socket.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c index
> b3b72d8e9543..ea61c32f6b80 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -2756,8 +2756,10 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct
> tipc_group_req *mreq)
> seq.upper = seq.lower;
> tipc_nametbl_build_group(net, grp, mreq->type, domain);
> rc = tipc_sk_publish(tsk, mreq->scope, &seq);
> - if (rc)
> + if (rc) {
> tipc_group_delete(net, grp);
> + tsk->group = NULL;
> + }
>
> /* Eliminate any risk that a broadcast overtakes the sent JOIN */
> tsk->mc_method.rcast = true;
> --
> 2.13.0
^ permalink raw reply
* Re: [patch net-next v2 00/20] net: sched: convert cls ndo_setup_tc offload calls to per-block callbacks
From: Jiri Pirko @ 2017-10-25 8:36 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Jamal Hadi Salim, Cong Wang, mlxsw,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Michael Chan,
Ganesh Goudar, Jeff Kirsher, Saeed Mahameed, Matan Barak,
Leon Romanovsky, idosch, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Simon Horman, pieter.jansenvanvuuren, john.hurl
In-Reply-To: <CAKgT0UeR8t3C82cQo9er4zkQy7PH36H4r84WA7V7HLOavY7RGg@mail.gmail.com>
Tue, Oct 24, 2017 at 06:24:01PM CEST, alexander.duyck@gmail.com wrote:
>On Tue, Oct 24, 2017 at 9:01 AM, Alexander Duyck
><alexander.duyck@gmail.com> wrote:
>> On Fri, Oct 20, 2017 at 7:04 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Jiri Pirko <jiri@resnulli.us>
>>> Date: Thu, 19 Oct 2017 15:50:28 +0200
>>>
>>>> This patchset is a bit bigger, but most of the patches are doing the
>>>> same changes in multiple classifiers and drivers. I could do some
>>>> squashes, but I think it is better split.
>>>>
>>>> This is another dependency on the way to shared block implementation.
>>>> The goal is to remove use of tp->q in classifiers code.
>>>>
>>>> Also, this provides drivers possibility to track binding of blocks to
>>>> qdiscs. Legacy drivers which do not support shared block offloading.
>>>> register one callback per binding. That maintains the current
>>>> functionality we have with ndo_setup_tc. Drivers which support block
>>>> sharing offload register one callback per block which safes overhead.
>>>>
>>>> Patches 1-4 introduce the binding notifications and per-block callbacks
>>>> Patches 5-8 add block callbacks calls to classifiers
>>>> Patches 9-17 do convert from ndo_setup_tc calls to block callbacks for
>>>> classifier offloads in drivers
>>>> Patches 18-20 do cleanup
>>>
>>> Series applied.
>>
>> We are getting internal reports of regressions being seen with this
>> patch set applied. Specifically the issues below have been pointed out
>> to me. My understanding is that these issues are all being reported on
>> ixgbe:
>>
>> 1. To offload filter into HW, the hw-tc-offload feature flag has
>> to be turned on before creating the ingress qdisc.
>>
>> Previously, this could also be turned on after the qdisc was created
>> and the filters could still be offloaded. Looks like this is because,
>> previously the offload flag was checked as a part of filter
>> integration in the classifier, and now it is checked as part of qdisc
>> creation (ingress_init). So, if no offload capability is advertised at
>> ingress qdisc creation time then hardware will not be asked to offload
>> filters later if the flag is enabled.
>>
>> 2. Deleting the ingress qdisc fails to remove filters added in
>> HW. Filters in SW gets deleted.
>>
>> We haven’t exactly root-caused this, the changes being extensive, but
>> our guess is again something wrong with the offload check or similar
>> while unregistering the block callback (tcf_block_cb_unregister) and
>> further to the classifier (CLS_U32/CLS_FLOWER etc.) with the
>> DESTROY/REMOVE command.
>>
>> 3. Deleting u32 filters using handle fails to remove filter from
>> HW, filter in SW gets deleted.
>>
>> Probably similar reasons, also we see some u32 specific patches as
>> well for remove nodes.
>>
>> We are still digging into this further, but wanted to put this out
>> there so we can address the issues before we go much further down this
>> path.
>>
>> Thanks.
>>
>> - Alex
>
>So a quick update. Item 3 is no longer an issue, it was a
>configuration issue on our side. So only items 1 and 2 are left to be
>addressed.
Will look at 1 and 2. Thanks for the report.
>
>Thanks.
>
>- Alex
^ permalink raw reply
* [PATCH V3 net-next 1/3] smc: fix mutex unlocks during link group creation
From: Ursula Braun @ 2017-10-25 9:01 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, jwi, schwidefsky, heiko.carstens, raspl,
ubraun
In-Reply-To: <20171025090146.83471-1-ubraun@linux.vnet.ibm.com>
Link group creation is synchronized with the smc_create_lgr_pending
lock. In smc_listen_work() this mutex is sometimes unlocked, even
though it has not been locked before. This issue will surface in
presence of the SMC rendezvous code.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/af_smc.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 745f145d4c4d..70f7640f5090 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -808,7 +808,7 @@ static void smc_listen_work(struct work_struct *work)
rc = local_contact;
if (rc == -ENOMEM)
reason_code = SMC_CLC_DECL_MEM;/* insufficient memory*/
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];
@@ -816,7 +816,7 @@ static void smc_listen_work(struct work_struct *work)
rc = smc_buf_create(new_smc);
if (rc) {
reason_code = SMC_CLC_DECL_MEM;
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
smc_close_init(new_smc);
@@ -831,7 +831,7 @@ static void smc_listen_work(struct work_struct *work)
buf_desc->mr_rx[SMC_SINGLE_LINK]);
if (rc) {
reason_code = SMC_CLC_DECL_INTERR;
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
}
}
@@ -839,15 +839,15 @@ static void smc_listen_work(struct work_struct *work)
rc = smc_clc_send_accept(new_smc, local_contact);
if (rc)
- goto out_err;
+ goto out_err_unlock;
/* receive SMC Confirm CLC message */
reason_code = smc_clc_wait_msg(new_smc, &cclc, sizeof(cclc),
SMC_CLC_CONFIRM);
if (reason_code < 0)
- goto out_err;
+ goto out_err_unlock;
if (reason_code > 0)
- goto decline_rdma;
+ goto decline_rdma_unlock;
smc_conn_save_peer_info(new_smc, &cclc);
if (local_contact == SMC_FIRST_CONTACT)
smc_link_save_peer_info(link, &cclc);
@@ -855,34 +855,34 @@ static void smc_listen_work(struct work_struct *work)
rc = smc_rmb_rtoken_handling(&new_smc->conn, &cclc);
if (rc) {
reason_code = SMC_CLC_DECL_INTERR;
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
if (local_contact == SMC_FIRST_CONTACT) {
rc = smc_ib_ready_link(link);
if (rc) {
reason_code = SMC_CLC_DECL_INTERR;
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
/* QP confirmation over RoCE fabric */
reason_code = smc_serv_conf_first_link(new_smc);
if (reason_code < 0) {
/* peer is not aware of a problem */
rc = reason_code;
- goto out_err;
+ goto out_err_unlock;
}
if (reason_code > 0)
- goto decline_rdma;
+ goto decline_rdma_unlock;
}
smc_tx_init(new_smc);
+ mutex_unlock(&smc_create_lgr_pending);
out_connected:
sk_refcnt_debug_inc(newsmcsk);
if (newsmcsk->sk_state == SMC_INIT)
newsmcsk->sk_state = SMC_ACTIVE;
enqueue:
- mutex_unlock(&smc_create_lgr_pending);
lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
if (lsmc->sk.sk_state == SMC_LISTEN) {
smc_accept_enqueue(&lsmc->sk, newsmcsk);
@@ -896,6 +896,8 @@ static void smc_listen_work(struct work_struct *work)
sock_put(&lsmc->sk); /* sock_hold in smc_tcp_listen_work */
return;
+decline_rdma_unlock:
+ mutex_unlock(&smc_create_lgr_pending);
decline_rdma:
/* RDMA setup failed, switch back to TCP */
smc_conn_free(&new_smc->conn);
@@ -907,6 +909,8 @@ static void smc_listen_work(struct work_struct *work)
}
goto out_connected;
+out_err_unlock:
+ mutex_unlock(&smc_create_lgr_pending);
out_err:
newsmcsk->sk_state = SMC_CLOSED;
smc_conn_free(&new_smc->conn);
--
2.13.5
^ permalink raw reply related
* [PATCH V3 net-next 3/3] smc: add SMC rendezvous protocol
From: Ursula Braun @ 2017-10-25 9:01 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, jwi, schwidefsky, heiko.carstens, raspl,
ubraun
In-Reply-To: <20171025090146.83471-1-ubraun@linux.vnet.ibm.com>
The SMC protocol [1] uses a rendezvous protocol to negotiate SMC
capability between peers. The current Linux implementation does not yet
use this rendezvous protocol and, thus, is not compliant to RFC7609 and
incompatible with other SMC implementations like in zOS.
This patch adds support for the SMC rendezvous protocol. It uses a new
TCP experimental option. With this option, SMC capabilities are
exchanged between the peers during the TCP three way handshake.
[1] SMC-R Informational RFC: http://www.rfc-editor.org/info/rfc7609
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/af_smc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 70f7640f5090..6451c5013e06 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -390,6 +390,12 @@ static int smc_connect_rdma(struct smc_sock *smc)
int rc = 0;
u8 ibport;
+ if (!tcp_sk(smc->clcsock->sk)->syn_smc) {
+ /* peer has not signalled SMC-capability */
+ smc->use_fallback = true;
+ goto out_connected;
+ }
+
/* IPSec connections opt out of SMC-R optimizations */
if (using_ipsec(smc)) {
reason_code = SMC_CLC_DECL_IPSEC;
@@ -555,6 +561,7 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
}
smc_copy_sock_settings_to_clc(smc);
+ tcp_sk(smc->clcsock->sk)->syn_smc = 1;
rc = kernel_connect(smc->clcsock, addr, alen, flags);
if (rc)
goto out;
@@ -759,6 +766,12 @@ static void smc_listen_work(struct work_struct *work)
u8 prefix_len;
u8 ibport;
+ /* check if peer is smc capable */
+ if (!tcp_sk(newclcsock->sk)->syn_smc) {
+ new_smc->use_fallback = true;
+ goto out_connected;
+ }
+
/* do inband token exchange -
*wait for and receive SMC Proposal CLC message
*/
@@ -967,6 +980,7 @@ static int smc_listen(struct socket *sock, int backlog)
* them to the clc socket -- copy smc socket options to clc socket
*/
smc_copy_sock_settings_to_clc(smc);
+ tcp_sk(smc->clcsock->sk)->syn_smc = 1;
rc = kernel_listen(smc->clcsock, backlog);
if (rc)
@@ -1409,6 +1423,7 @@ static int __init smc_init(void)
goto out_sock;
}
+ static_branch_enable(&tcp_have_smc);
return 0;
out_sock:
@@ -1433,6 +1448,7 @@ static void __exit smc_exit(void)
list_del_init(&lgr->list);
smc_lgr_free(lgr); /* free link group */
}
+ static_branch_disable(&tcp_have_smc);
smc_ib_unregister_client();
sock_unregister(PF_SMC);
proto_unregister(&smc_proto);
--
2.13.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox