* Re: [PATCH 0/4] RCU: introduce noref debug
From: Paul E. McKenney @ 2017-10-11 4:02 UTC (permalink / raw)
To: Paolo Abeni
Cc: linux-kernel, Josh Triplett, Steven Rostedt, David S. Miller,
Eric Dumazet, Hannes Frederic Sowa, netdev
In-Reply-To: <1507567992.21825.9.camel@redhat.com>
On Mon, Oct 09, 2017 at 06:53:12PM +0200, Paolo Abeni wrote:
> On Fri, 2017-10-06 at 09:34 -0700, Paul E. McKenney wrote:
> > On Fri, Oct 06, 2017 at 05:10:09PM +0200, Paolo Abeni wrote:
> > > Hi,
> > >
> > > On Fri, 2017-10-06 at 06:34 -0700, Paul E. McKenney wrote:
> > > > On Fri, Oct 06, 2017 at 02:57:45PM +0200, Paolo Abeni wrote:
> > > > > The networking subsystem is currently using some kind of long-lived
> > > > > RCU-protected, references to avoid the overhead of full book-keeping.
> > > > >
> > > > > Such references - skb_dst() noref - are stored inside the skbs and can be
> > > > > moved across relevant slices of the network stack, with the users
> > > > > being in charge of properly clearing the relevant skb - or properly refcount
> > > > > the related dst references - before the skb escapes the RCU section.
> > > > >
> > > > > We currently don't have any deterministic debug infrastructure to check
> > > > > the dst noref usages - and the introduction of others noref artifact is
> > > > > currently under discussion.
> > > > >
> > > > > This series tries to tackle the above introducing an RCU debug infrastructure
> > > > > aimed at spotting incorrect noref pointer usage, in patch one. The
> > > > > infrastructure is small and must be explicitly enabled via a newly introduced
> > > > > build option.
> > > > >
> > > > > Patch two uses such infrastructure to track dst noref usage in the networking
> > > > > stack.
> > > > >
> > > > > Patch 3 and 4 are bugfixes for small buglet found running this infrastructure
> > > > > on basic scenarios.
> > >
> > > Thank you for the prompt reply!
> > > >
> > > > This patchset does not look like it handles rcu_read_lock() nesting.
> > > > For example, given code like this:
> > > >
> > > > void foo(void)
> > > > {
> > > > rcu_read_lock();
> > > > rcu_track_noref(&key2, &noref2, true);
> > > > do_something();
> > > > rcu_track_noref(&key2, &noref2, false);
> > > > rcu_read_unlock();
> > > > }
> > > >
> > > > void bar(void)
> > > > {
> > > > rcu_read_lock();
> > > > rcu_track_noref(&key1, &noref1, true);
> > > > do_something_more();
> > > > foo();
> > > > do_something_else();
> > > > rcu_track_noref(&key1, &noref1, false);
> > > > rcu_read_unlock();
> > > > }
> > > >
> > > > void grill(void)
> > > > {
> > > > foo();
> > > > }
> > > >
> > > > It looks like foo()'s rcu_read_unlock() will complain about key1.
> > > > You could remove foo()'s rcu_read_lock() and rcu_read_unlock(), but
> > > > that will break the call from grill().
> > >
> > > Actually the code should cope correctly with your example; when foo()'s
> > > rcu_read_unlock() is called, 'cache' contains:
> > >
> > > { { &key1, &noref1, 1}, // ...
> > >
> > > and when the related __rcu_check_noref() is invoked preempt_count() is
> > > 2 - because the check is called before decreasing the preempt counter.
> > >
> > > In the main loop inside __rcu_check_noref() we will hit always the
> > > 'continue' statement because 'cache->store[i].nesting != nesting', so
> > > no warn will be triggered.
> >
> > You are right, it was too early, and my example wasn't correct. How
> > about this one?
> >
> > void foo(void (*f)(struct s *sp), struct s **spp)
> > {
> > rcu_read_lock();
> > rcu_track_noref(&key2, &noref2, true);
> > f(spp);
> > rcu_track_noref(&key2, &noref2, false);
> > rcu_read_unlock();
> > }
> >
> > void barcb(struct s **spp)
> > {
> > *spp = &noref3;
> > rcu_track_noref(&key3, *spp, true);
> > }
> >
> > void bar(void)
> > {
> > struct s *sp;
> >
> > rcu_read_lock();
> > rcu_track_noref(&key1, &noref1, true);
> > do_something_more();
> > foo(barcb, &sp);
> > do_something_else(sp);
> > rcu_track_noref(&key3, sp, false);
> > rcu_track_noref(&key1, &noref1, false);
> > rcu_read_unlock();
> > }
> >
> > void grillcb(struct s **spp)
> > {
> > *spp
> > }
> >
> > void grill(void)
> > {
> > foo();
> > }
>
> You are right: this will generate a splat, even if the code it safe.
> The false positive can be avoided looking for leaked references only in
> the outermost rcu unlook. I did a previous implementation performing
> such check, but it emitted very generic splat so I tried to be more
> strict. The latter choice allowed to find/do 3/4.
>
> What about using save_stack_trace() in rcu_track_noref(, true) and
> reporting such stack trace when the check in the outer most rcu fails?
>
> the current strict/false-postive-prone check could be enabled under an
> additional build flag.
Linus and Ingo will ask me how users decide how they should set that
additional build flag. Especially given that if there is code that
requires non-strict checking, isn't everyone required to set up non-strict
checking to avoid false positives? Unless you can also configure out
all the code that requires non-strict checking, I suppose, but how
would you keep track of what to configure out?
> > How does the user select the key argument? It looks like someone
> > can choose to just pass in NULL -- is that the intent, or am I confused
> > about this as well?
>
> The 'key' argument is intented to discriminate the scope of the same
> noref dst attached to different skbs, which happens e.g. as a result of
> as skb_dst_copy().
>
> In a generic use case, it can be NULL, too.
OK. There will probably be some discussion about the API in that case.
> > I am also concerned about false negatives when the user invokes
> > rcu_track_noref(..., false) but then leaks the pointer anyway. Or is
> > there something you are doing that catches this that I am missing?
>
> If the rcu_track_noref(..., false) is misplaced or missing, yes we can
> have false negative.
>
> In the noref rcu use-case the rcu_track_noref(, false) call is/must be
> placed side-by-side with the code clearing the the noref pointer, so
> is/should be quite easy avoiding such mistakes.
True enough. Except that if people were good about always clearing the
pointer, then the pointer couldn't leak, right? Or am I missing something
in your use cases?
Or to put it another way -- have you been able to catch any real
pointer-leak bugs with this? The other RCU debug options have had
pretty long found-bug lists before we accepted them.
Thanx, Paul
^ permalink raw reply
* (unknown),
From: morice.diane @ 2017-10-11 4:11 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 998537216140305.zip --]
[-- Type: application/zip, Size: 6055 bytes --]
^ permalink raw reply
* [PATCH net-next 0/7] Rewrite some existing functionality
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
This series fixes some of the broken rmnet functionality.
Bridge mode is re-written and made useable and the muxed_ep is converted to hlist.
Patches 1-5 are cleanups in preparation for these changes.
Patch 6 does the hlist conversion.
Patch 7 has the implementation of the rmnet bridge mode.
Subash Abhinov Kasiviswanathan (7):
net: qualcomm: rmnet: Remove existing logic for bridge mode
net: qualcomm: rmnet: Remove some unused defines
net: qualcomm: rmnet: Move rmnet_mode to rmnet_port
net: qualcomm: rmnet: Remove duplicate setting of rmnet private info
net: qualcomm: rmnet: Remove duplicate setting of rmnet_devices
net: qualcomm: rmnet: Convert the muxed endpoint to hlist
net: qualcomm: rmnet: Implement bridge mode
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 166 ++++++++++++++++-----
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 19 +--
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 131 ++++++----------
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.h | 3 +-
.../ethernet/qualcomm/rmnet/rmnet_map_command.c | 4 +-
.../net/ethernet/qualcomm/rmnet/rmnet_private.h | 8 -
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 36 ++---
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 7 +-
8 files changed, 204 insertions(+), 170 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next 1/7] net: qualcomm: rmnet: Remove existing logic for bridge mode
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
This will be rewritten in the following patches.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 1 -
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 77 +++-------------------
2 files changed, 9 insertions(+), 69 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index dde4e9f..0b0c5a7 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -34,7 +34,6 @@ struct rmnet_endpoint {
*/
struct rmnet_port {
struct net_device *dev;
- struct rmnet_endpoint local_ep;
struct rmnet_endpoint muxed_ep[RMNET_MAX_LOGICAL_EP];
u32 ingress_data_format;
u32 egress_data_format;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 540c762..b50f401 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -44,56 +44,18 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
/* Generic handler */
static rx_handler_result_t
-rmnet_bridge_handler(struct sk_buff *skb, struct rmnet_endpoint *ep)
+rmnet_deliver_skb(struct sk_buff *skb)
{
- if (!ep->egress_dev)
- kfree_skb(skb);
- else
- rmnet_egress_handler(skb, ep);
+ 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);
return RX_HANDLER_CONSUMED;
}
-static rx_handler_result_t
-rmnet_deliver_skb(struct sk_buff *skb, struct rmnet_endpoint *ep)
-{
- switch (ep->rmnet_mode) {
- case RMNET_EPMODE_NONE:
- return RX_HANDLER_PASS;
-
- case RMNET_EPMODE_BRIDGE:
- return rmnet_bridge_handler(skb, ep);
-
- case RMNET_EPMODE_VND:
- 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);
- return RX_HANDLER_CONSUMED;
-
- default:
- kfree_skb(skb);
- return RX_HANDLER_CONSUMED;
- }
-}
-
-static rx_handler_result_t
-rmnet_ingress_deliver_packet(struct sk_buff *skb,
- struct rmnet_port *port)
-{
- if (!port) {
- kfree_skb(skb);
- return RX_HANDLER_CONSUMED;
- }
-
- skb->dev = port->local_ep.egress_dev;
-
- return rmnet_deliver_skb(skb, &port->local_ep);
-}
-
/* MAP handler */
static rx_handler_result_t
@@ -130,7 +92,7 @@ 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, ep);
+ return rmnet_deliver_skb(skb);
}
static rx_handler_result_t
@@ -204,29 +166,8 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
dev = skb->dev;
port = rmnet_get_port(dev);
- if (port->ingress_data_format & RMNET_INGRESS_FORMAT_MAP) {
+ if (port->ingress_data_format & RMNET_INGRESS_FORMAT_MAP)
rc = rmnet_map_ingress_handler(skb, port);
- } else {
- switch (ntohs(skb->protocol)) {
- case ETH_P_MAP:
- if (port->local_ep.rmnet_mode ==
- RMNET_EPMODE_BRIDGE) {
- rc = rmnet_ingress_deliver_packet(skb, port);
- } else {
- kfree_skb(skb);
- rc = RX_HANDLER_CONSUMED;
- }
- break;
-
- case ETH_P_IP:
- case ETH_P_IPV6:
- rc = rmnet_ingress_deliver_packet(skb, port);
- break;
-
- default:
- rc = RX_HANDLER_PASS;
- }
- }
return rc;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/7] net: qualcomm: rmnet: Remove some unused defines
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
Most of these constants were used in the initial patchset where
custom netlink configuration was used and hence are no longer relevant.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
index 7967198..49102f9 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
@@ -19,23 +19,15 @@
#define RMNET_TX_QUEUE_LEN 1000
/* Constants */
-#define RMNET_EGRESS_FORMAT__RESERVED__ BIT(0)
#define RMNET_EGRESS_FORMAT_MAP BIT(1)
#define RMNET_EGRESS_FORMAT_AGGREGATION BIT(2)
#define RMNET_EGRESS_FORMAT_MUXING BIT(3)
-#define RMNET_EGRESS_FORMAT_MAP_CKSUMV3 BIT(4)
-#define RMNET_EGRESS_FORMAT_MAP_CKSUMV4 BIT(5)
-#define RMNET_INGRESS_FIX_ETHERNET BIT(0)
#define RMNET_INGRESS_FORMAT_MAP BIT(1)
#define RMNET_INGRESS_FORMAT_DEAGGREGATION BIT(2)
#define RMNET_INGRESS_FORMAT_DEMUXING BIT(3)
#define RMNET_INGRESS_FORMAT_MAP_COMMANDS BIT(4)
-#define RMNET_INGRESS_FORMAT_MAP_CKSUMV3 BIT(5)
-#define RMNET_INGRESS_FORMAT_MAP_CKSUMV4 BIT(6)
-/* Pass the frame up the stack with no modifications to skb->dev */
-#define RMNET_EPMODE_NONE (0)
/* Replace skb->dev to a virtual rmnet device and pass up the stack */
#define RMNET_EPMODE_VND (1)
/* Pass the frame directly to another device with dev_queue_xmit() */
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 3/7] net: qualcomm: rmnet: Move rmnet_mode to rmnet_port
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
Mode information on the real device makes it easier to route packets
to rmnet device or bridged device based on the configuration.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 12 +++++-------
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 2 +-
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 3 +--
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 8403eea..85fce9c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -124,20 +124,17 @@ static int rmnet_register_real_device(struct net_device *real_dev)
}
static void rmnet_set_endpoint_config(struct net_device *dev,
- u8 mux_id, u8 rmnet_mode,
- struct net_device *egress_dev)
+ u8 mux_id, struct net_device *egress_dev)
{
struct rmnet_endpoint *ep;
- netdev_dbg(dev, "id %d mode %d dev %s\n",
- mux_id, rmnet_mode, egress_dev->name);
+ netdev_dbg(dev, "id %d dev %s\n", mux_id, egress_dev->name);
ep = rmnet_get_endpoint(dev, mux_id);
/* This config is cleared on every set, so its ok to not
* clear it on a device delete.
*/
memset(ep, 0, sizeof(struct rmnet_endpoint));
- ep->rmnet_mode = rmnet_mode;
ep->egress_dev = egress_dev;
ep->mux_id = mux_id;
}
@@ -183,9 +180,10 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
ingress_format, egress_format);
port->egress_data_format = egress_format;
port->ingress_data_format = ingress_format;
+ port->rmnet_mode = mode;
- rmnet_set_endpoint_config(real_dev, mux_id, mode, dev);
- rmnet_set_endpoint_config(dev, mux_id, mode, real_dev);
+ rmnet_set_endpoint_config(real_dev, mux_id, dev);
+ rmnet_set_endpoint_config(dev, mux_id, real_dev);
return 0;
err2:
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 0b0c5a7..03d473f 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -24,7 +24,6 @@
* Exact usage of this parameter depends on the rmnet_mode.
*/
struct rmnet_endpoint {
- u8 rmnet_mode;
u8 mux_id;
struct net_device *egress_dev;
};
@@ -39,6 +38,7 @@ struct rmnet_port {
u32 egress_data_format;
struct net_device *rmnet_devices[RMNET_MAX_LOGICAL_EP];
u8 nr_rmnet_devs;
+ u8 rmnet_mode;
};
extern struct rtnl_link_ops rmnet_link_ops;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index b50f401..86e37cc 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -205,8 +205,7 @@ void rmnet_egress_handler(struct sk_buff *skb,
}
}
- if (ep->rmnet_mode == RMNET_EPMODE_VND)
- rmnet_vnd_tx_fixup(skb, orig_dev);
+ rmnet_vnd_tx_fixup(skb, orig_dev);
dev_queue_xmit(skb);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 4/7] net: qualcomm: rmnet: Remove duplicate setting of rmnet private info
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
The end point is set twice in the local_ep as well as the mux_id and
the real_dev in the rmnet private structure. Remove the local_ep.
While these elements are equivalent, rmnet_endpoint will be
used only as part of the rmnet_port for muxed scenarios in VND mode.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 10 ++--------
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 ----
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 18 ++++++++++--------
drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.h | 3 +--
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 19 ++-----------------
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 1 -
6 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 85fce9c..96058bb 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -67,13 +67,8 @@ static int rmnet_is_real_dev_registered(const struct net_device *real_dev)
struct rmnet_endpoint *ep;
struct rmnet_port *port;
- if (!rmnet_is_real_dev_registered(dev)) {
- ep = rmnet_vnd_get_endpoint(dev);
- } else {
- port = rmnet_get_port_rtnl(dev);
-
- ep = &port->muxed_ep[config_id];
- }
+ port = rmnet_get_port_rtnl(dev);
+ ep = &port->muxed_ep[config_id];
return ep;
}
@@ -183,7 +178,6 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
port->rmnet_mode = mode;
rmnet_set_endpoint_config(real_dev, mux_id, dev);
- rmnet_set_endpoint_config(dev, mux_id, real_dev);
return 0;
err2:
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 03d473f..c5f5c6d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -20,9 +20,6 @@
#define RMNET_MAX_LOGICAL_EP 255
-/* Information about the next device to deliver the packet to.
- * Exact usage of this parameter depends on the rmnet_mode.
- */
struct rmnet_endpoint {
u8 mux_id;
struct net_device *egress_dev;
@@ -44,7 +41,6 @@ struct rmnet_port {
extern struct rtnl_link_ops rmnet_link_ops;
struct rmnet_priv {
- struct rmnet_endpoint local_ep;
u8 mux_id;
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 86e37cc..e0802d3 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -116,8 +116,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
}
static int rmnet_map_egress_handler(struct sk_buff *skb,
- struct rmnet_port *port,
- struct rmnet_endpoint *ep,
+ struct rmnet_port *port, u8 mux_id,
struct net_device *orig_dev)
{
int required_headroom, additional_header_len;
@@ -136,10 +135,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
return RMNET_MAP_CONSUMED;
if (port->egress_data_format & RMNET_EGRESS_FORMAT_MUXING) {
- if (ep->mux_id == 0xff)
+ if (mux_id == 0xff)
map_header->mux_id = 0;
else
- map_header->mux_id = ep->mux_id;
+ map_header->mux_id = mux_id;
}
skb->protocol = htons(ETH_P_MAP);
@@ -176,14 +175,17 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
* for egress device configured in logical endpoint. Packet is then transmitted
* on the egress device.
*/
-void rmnet_egress_handler(struct sk_buff *skb,
- struct rmnet_endpoint *ep)
+void rmnet_egress_handler(struct sk_buff *skb)
{
struct net_device *orig_dev;
struct rmnet_port *port;
+ struct rmnet_priv *priv;
+ u8 mux_id;
orig_dev = skb->dev;
- skb->dev = ep->egress_dev;
+ priv = netdev_priv(orig_dev);
+ skb->dev = priv->real_dev;
+ mux_id = priv->mux_id;
port = rmnet_get_port(skb->dev);
if (!port) {
@@ -192,7 +194,7 @@ void rmnet_egress_handler(struct sk_buff *skb,
}
if (port->egress_data_format & RMNET_EGRESS_FORMAT_MAP) {
- switch (rmnet_map_egress_handler(skb, port, ep, orig_dev)) {
+ switch (rmnet_map_egress_handler(skb, port, mux_id, orig_dev)) {
case RMNET_MAP_CONSUMED:
return;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.h
index f2638cf..3537e4c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.h
@@ -18,8 +18,7 @@
#include "rmnet_config.h"
-void rmnet_egress_handler(struct sk_buff *skb,
- struct rmnet_endpoint *ep);
+void rmnet_egress_handler(struct sk_buff *skb);
rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 7f90d55..4ca59a4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -45,8 +45,8 @@ static netdev_tx_t rmnet_vnd_start_xmit(struct sk_buff *skb,
struct rmnet_priv *priv;
priv = netdev_priv(dev);
- if (priv->local_ep.egress_dev) {
- rmnet_egress_handler(skb, &priv->local_ep);
+ if (priv->real_dev) {
+ rmnet_egress_handler(skb);
} else {
dev->stats.tx_dropped++;
kfree_skb(skb);
@@ -143,21 +143,6 @@ u8 rmnet_vnd_get_mux(struct net_device *rmnet_dev)
return priv->mux_id;
}
-/* Gets the logical endpoint configuration for a RmNet virtual network device
- * node. Caller should confirm that devices is a RmNet VND before calling.
- */
-struct rmnet_endpoint *rmnet_vnd_get_endpoint(struct net_device *rmnet_dev)
-{
- struct rmnet_priv *priv;
-
- if (!rmnet_dev)
- return NULL;
-
- priv = netdev_priv(rmnet_dev);
-
- return &priv->local_ep;
-}
-
int rmnet_vnd_do_flow_control(struct net_device *rmnet_dev, int enable)
{
netdev_dbg(rmnet_dev, "Setting VND TX queue state to %d\n", enable);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
index 8a4042f..cae134d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
@@ -17,7 +17,6 @@
#define _RMNET_VND_H_
int rmnet_vnd_do_flow_control(struct net_device *dev, int enable);
-struct rmnet_endpoint *rmnet_vnd_get_endpoint(struct net_device *dev);
int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
struct rmnet_port *port,
struct net_device *real_dev);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 5/7] net: qualcomm: rmnet: Remove duplicate setting of rmnet_devices
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
The rmnet_devices information is already stored in muxed_ep, so
storing this in rmnet_devices[] again is redundant.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 1 -
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 8 ++++----
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index c5f5c6d..123ccf4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -33,7 +33,6 @@ struct rmnet_port {
struct rmnet_endpoint muxed_ep[RMNET_MAX_LOGICAL_EP];
u32 ingress_data_format;
u32 egress_data_format;
- struct net_device *rmnet_devices[RMNET_MAX_LOGICAL_EP];
u8 nr_rmnet_devs;
u8 rmnet_mode;
};
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 4ca59a4..8b8497b 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -105,12 +105,12 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
struct rmnet_priv *priv;
int rc;
- if (port->rmnet_devices[id])
+ if (port->muxed_ep[id].egress_dev)
return -EINVAL;
rc = register_netdevice(rmnet_dev);
if (!rc) {
- port->rmnet_devices[id] = rmnet_dev;
+ port->muxed_ep[id].egress_dev = rmnet_dev;
port->nr_rmnet_devs++;
rmnet_dev->rtnl_link_ops = &rmnet_link_ops;
@@ -127,10 +127,10 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
int rmnet_vnd_dellink(u8 id, struct rmnet_port *port)
{
- if (id >= RMNET_MAX_LOGICAL_EP || !port->rmnet_devices[id])
+ if (id >= RMNET_MAX_LOGICAL_EP || !port->muxed_ep[id].egress_dev)
return -EINVAL;
- port->rmnet_devices[id] = NULL;
+ port->muxed_ep[id].egress_dev = NULL;
port->nr_rmnet_devs--;
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 6/7] net: qualcomm: rmnet: Convert the muxed endpoint to hlist
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan, Dan Williams
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
Rather than using a static array, use a hlist to store the muxed
endpoints and use the mux id to query the rmnet_device.
This is useful as usually very few mux ids are used.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Cc: Dan Williams <dcbw@redhat.com>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 75 ++++++++++++----------
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 4 +-
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 17 +++--
.../ethernet/qualcomm/rmnet/rmnet_map_command.c | 4 +-
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 15 +++--
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h | 6 +-
6 files changed, 68 insertions(+), 53 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 96058bb..b5fe3f4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -61,18 +61,6 @@ static int rmnet_is_real_dev_registered(const struct net_device *real_dev)
return rtnl_dereference(real_dev->rx_handler_data);
}
-static struct rmnet_endpoint*
-rmnet_get_endpoint(struct net_device *dev, int config_id)
-{
- struct rmnet_endpoint *ep;
- struct rmnet_port *port;
-
- port = rmnet_get_port_rtnl(dev);
- ep = &port->muxed_ep[config_id];
-
- return ep;
-}
-
static int rmnet_unregister_real_device(struct net_device *real_dev,
struct rmnet_port *port)
{
@@ -93,7 +81,7 @@ static int rmnet_unregister_real_device(struct net_device *real_dev,
static int rmnet_register_real_device(struct net_device *real_dev)
{
struct rmnet_port *port;
- int rc;
+ int rc, entry;
ASSERT_RTNL();
@@ -114,26 +102,13 @@ static int rmnet_register_real_device(struct net_device *real_dev)
/* hold on to real dev for MAP data */
dev_hold(real_dev);
+ for (entry = 0; entry < RMNET_MAX_LOGICAL_EP; entry++)
+ INIT_HLIST_HEAD(&port->muxed_ep[entry]);
+
netdev_dbg(real_dev, "registered with rmnet\n");
return 0;
}
-static void rmnet_set_endpoint_config(struct net_device *dev,
- u8 mux_id, struct net_device *egress_dev)
-{
- struct rmnet_endpoint *ep;
-
- netdev_dbg(dev, "id %d dev %s\n", mux_id, egress_dev->name);
-
- ep = rmnet_get_endpoint(dev, mux_id);
- /* This config is cleared on every set, so its ok to not
- * clear it on a device delete.
- */
- memset(ep, 0, sizeof(struct rmnet_endpoint));
- ep->egress_dev = egress_dev;
- ep->mux_id = mux_id;
-}
-
static int rmnet_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[],
struct netlink_ext_ack *extack)
@@ -145,6 +120,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
RMNET_EGRESS_FORMAT_MAP;
struct net_device *real_dev;
int mode = RMNET_EPMODE_VND;
+ struct rmnet_endpoint *ep;
struct rmnet_port *port;
int err = 0;
u16 mux_id;
@@ -156,6 +132,10 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
if (!data[IFLA_VLAN_ID])
return -EINVAL;
+ ep = kzalloc(sizeof(*ep), GFP_ATOMIC);
+ if (!ep)
+ return -ENOMEM;
+
mux_id = nla_get_u16(data[IFLA_VLAN_ID]);
err = rmnet_register_real_device(real_dev);
@@ -163,7 +143,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
goto err0;
port = rmnet_get_port_rtnl(real_dev);
- err = rmnet_vnd_newlink(mux_id, dev, port, real_dev);
+ err = rmnet_vnd_newlink(mux_id, dev, port, real_dev, ep);
if (err)
goto err1;
@@ -177,11 +157,11 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
port->ingress_data_format = ingress_format;
port->rmnet_mode = mode;
- rmnet_set_endpoint_config(real_dev, mux_id, dev);
+ hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
return 0;
err2:
- rmnet_vnd_dellink(mux_id, port);
+ rmnet_vnd_dellink(mux_id, port, ep);
err1:
rmnet_unregister_real_device(real_dev, port);
err0:
@@ -191,6 +171,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
static void rmnet_dellink(struct net_device *dev, struct list_head *head)
{
struct net_device *real_dev;
+ struct rmnet_endpoint *ep;
struct rmnet_port *port;
u8 mux_id;
@@ -204,8 +185,15 @@ static void rmnet_dellink(struct net_device *dev, struct list_head *head)
port = rmnet_get_port_rtnl(real_dev);
mux_id = rmnet_vnd_get_mux(dev);
- rmnet_vnd_dellink(mux_id, port);
netdev_upper_dev_unlink(dev, real_dev);
+
+ ep = rmnet_get_endpoint(port, mux_id);
+ if (ep) {
+ hlist_del_init_rcu(&ep->hlnode);
+ rmnet_vnd_dellink(mux_id, port, ep);
+ kfree(ep);
+ }
+
rmnet_unregister_real_device(real_dev, port);
unregister_netdevice_queue(dev, head);
@@ -214,11 +202,16 @@ static void rmnet_dellink(struct net_device *dev, struct list_head *head)
static int rmnet_dev_walk_unreg(struct net_device *rmnet_dev, void *data)
{
struct rmnet_walk_data *d = data;
+ struct rmnet_endpoint *ep;
u8 mux_id;
mux_id = rmnet_vnd_get_mux(rmnet_dev);
-
- rmnet_vnd_dellink(mux_id, d->port);
+ ep = rmnet_get_endpoint(d->port, mux_id);
+ if (ep) {
+ hlist_del_init_rcu(&ep->hlnode);
+ rmnet_vnd_dellink(mux_id, d->port, ep);
+ kfree(ep);
+ }
netdev_upper_dev_unlink(rmnet_dev, d->real_dev);
unregister_netdevice_queue(rmnet_dev, d->head);
@@ -316,6 +309,18 @@ struct rmnet_port *rmnet_get_port(struct net_device *real_dev)
return NULL;
}
+struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id)
+{
+ struct rmnet_endpoint *ep;
+
+ hlist_for_each_entry_rcu(ep, &port->muxed_ep[mux_id], hlnode) {
+ if (ep->mux_id == mux_id)
+ return ep;
+ }
+
+ return NULL;
+}
+
/* Startup/Shutdown */
static int __init rmnet_init(void)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 123ccf4..8849986 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -23,6 +23,7 @@
struct rmnet_endpoint {
u8 mux_id;
struct net_device *egress_dev;
+ struct hlist_node hlnode;
};
/* One instance of this structure is instantiated for each real_dev associated
@@ -30,11 +31,11 @@ struct rmnet_endpoint {
*/
struct rmnet_port {
struct net_device *dev;
- struct rmnet_endpoint muxed_ep[RMNET_MAX_LOGICAL_EP];
u32 ingress_data_format;
u32 egress_data_format;
u8 nr_rmnet_devs;
u8 rmnet_mode;
+ struct hlist_head muxed_ep[RMNET_MAX_LOGICAL_EP];
};
extern struct rtnl_link_ops rmnet_link_ops;
@@ -45,5 +46,6 @@ struct rmnet_priv {
};
struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
+struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id);
#endif /* _RMNET_CONFIG_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index e0802d3..fa24ffb 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -71,19 +71,18 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
& RMNET_INGRESS_FORMAT_MAP_COMMANDS)
return rmnet_map_command(skb, port);
- kfree_skb(skb);
- return RX_HANDLER_CONSUMED;
+ goto free_skb;
}
mux_id = RMNET_MAP_GET_MUX_ID(skb);
len = RMNET_MAP_GET_LENGTH(skb) - RMNET_MAP_GET_PAD(skb);
- if (mux_id >= RMNET_MAX_LOGICAL_EP) {
- kfree_skb(skb);
- return RX_HANDLER_CONSUMED;
- }
+ if (mux_id >= RMNET_MAX_LOGICAL_EP)
+ goto free_skb;
- ep = &port->muxed_ep[mux_id];
+ ep = rmnet_get_endpoint(port, mux_id);
+ if (!ep)
+ goto free_skb;
if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEMUXING)
skb->dev = ep->egress_dev;
@@ -93,6 +92,10 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
skb_trim(skb, len);
rmnet_set_skb_proto(skb);
return rmnet_deliver_skb(skb);
+
+free_skb:
+ kfree_skb(skb);
+ return RX_HANDLER_CONSUMED;
}
static rx_handler_result_t
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
index d1ea5e2..74d362f 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
@@ -17,7 +17,7 @@
#include "rmnet_vnd.h"
static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
- struct rmnet_port *rdinfo,
+ struct rmnet_port *port,
int enable)
{
struct rmnet_map_control_command *cmd;
@@ -37,7 +37,7 @@ static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
return RX_HANDLER_CONSUMED;
}
- ep = &rdinfo->muxed_ep[mux_id];
+ ep = rmnet_get_endpoint(port, mux_id);
vnd = ep->egress_dev;
ip_family = cmd->flow_control.ip_family;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 8b8497b..1b6747d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -100,17 +100,19 @@ void rmnet_vnd_setup(struct net_device *rmnet_dev)
int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
struct rmnet_port *port,
- struct net_device *real_dev)
+ struct net_device *real_dev,
+ struct rmnet_endpoint *ep)
{
struct rmnet_priv *priv;
int rc;
- if (port->muxed_ep[id].egress_dev)
+ if (ep->egress_dev)
return -EINVAL;
rc = register_netdevice(rmnet_dev);
if (!rc) {
- port->muxed_ep[id].egress_dev = rmnet_dev;
+ ep->egress_dev = rmnet_dev;
+ ep->mux_id = id;
port->nr_rmnet_devs++;
rmnet_dev->rtnl_link_ops = &rmnet_link_ops;
@@ -125,12 +127,13 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
return rc;
}
-int rmnet_vnd_dellink(u8 id, struct rmnet_port *port)
+int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
+ struct rmnet_endpoint *ep)
{
- if (id >= RMNET_MAX_LOGICAL_EP || !port->muxed_ep[id].egress_dev)
+ if (id >= RMNET_MAX_LOGICAL_EP || !ep->egress_dev)
return -EINVAL;
- port->muxed_ep[id].egress_dev = NULL;
+ 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 cae134d..71e4c32 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
@@ -19,8 +19,10 @@
int rmnet_vnd_do_flow_control(struct net_device *dev, int enable);
int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
struct rmnet_port *port,
- struct net_device *real_dev);
-int rmnet_vnd_dellink(u8 id, struct rmnet_port *port);
+ struct net_device *real_dev,
+ struct rmnet_endpoint *ep);
+int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
+ struct rmnet_endpoint *ep);
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
* [PATCH net-next 7/7] net: qualcomm: rmnet: Implement bridge mode
From: Subash Abhinov Kasiviswanathan @ 2017-10-11 4:17 UTC (permalink / raw)
To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1507695456-17051-1-git-send-email-subashab@codeaurora.org>
Add support to bridge two devices which can send multiplexing and
aggregation (MAP) data. This is done only when the data itself is
not going to be consumed in the stack but is being passed on to a
different endpoint. This is mainly used for testing.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 93 +++++++++++++++++++++-
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h | 7 +-
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 18 +++++
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 2 +
4 files changed, 118 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index b5fe3f4..71bee1a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -109,6 +109,36 @@ static int rmnet_register_real_device(struct net_device *real_dev)
return 0;
}
+static void rmnet_unregister_bridge(struct net_device *dev,
+ struct rmnet_port *port)
+{
+ struct net_device *rmnet_dev, *bridge_dev;
+ struct rmnet_port *bridge_port;
+
+ if (port->rmnet_mode != RMNET_EPMODE_BRIDGE)
+ return;
+
+ /* bridge slave handling */
+ if (!port->nr_rmnet_devs) {
+ rmnet_dev = netdev_master_upper_dev_get_rcu(dev);
+ netdev_upper_dev_unlink(dev, rmnet_dev);
+
+ bridge_dev = port->bridge_ep;
+
+ bridge_port = rmnet_get_port_rtnl(bridge_dev);
+ bridge_port->bridge_ep = NULL;
+ bridge_port->rmnet_mode = RMNET_EPMODE_VND;
+ } else {
+ bridge_dev = port->bridge_ep;
+
+ bridge_port = rmnet_get_port_rtnl(bridge_dev);
+ rmnet_dev = netdev_master_upper_dev_get_rcu(bridge_dev);
+ netdev_upper_dev_unlink(bridge_dev, rmnet_dev);
+
+ rmnet_unregister_real_device(bridge_dev, bridge_port);
+ }
+}
+
static int rmnet_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[],
struct netlink_ext_ack *extack)
@@ -190,10 +220,10 @@ static void rmnet_dellink(struct net_device *dev, struct list_head *head)
ep = rmnet_get_endpoint(port, mux_id);
if (ep) {
hlist_del_init_rcu(&ep->hlnode);
+ rmnet_unregister_bridge(dev, port);
rmnet_vnd_dellink(mux_id, port, ep);
kfree(ep);
}
-
rmnet_unregister_real_device(real_dev, port);
unregister_netdevice_queue(dev, head);
@@ -237,6 +267,8 @@ static void rmnet_force_unassociate_device(struct net_device *dev)
d.port = port;
rcu_read_lock();
+ rmnet_unregister_bridge(dev, port);
+
netdev_walk_all_lower_dev_rcu(real_dev, rmnet_dev_walk_unreg, &d);
rcu_read_unlock();
unregister_netdevice_many(&list);
@@ -321,6 +353,65 @@ struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id)
return NULL;
}
+int rmnet_add_bridge(struct net_device *rmnet_dev,
+ struct net_device *slave_dev,
+ struct netlink_ext_ack *extack)
+{
+ struct rmnet_priv *priv = netdev_priv(rmnet_dev);
+ struct net_device *real_dev = priv->real_dev;
+ struct rmnet_port *port, *slave_port;
+ int err;
+
+ port = rmnet_get_port(real_dev);
+
+ /* If there is more than one rmnet dev attached, its probably being
+ * used for muxing. Skip the briding in that case
+ */
+ if (port->nr_rmnet_devs > 1)
+ return -EINVAL;
+
+ if (rmnet_is_real_dev_registered(slave_dev))
+ return -EBUSY;
+
+ err = rmnet_register_real_device(slave_dev);
+ if (err)
+ return -EBUSY;
+
+ err = netdev_master_upper_dev_link(slave_dev, rmnet_dev, NULL, NULL,
+ extack);
+ if (err)
+ return -EINVAL;
+
+ slave_port = rmnet_get_port(slave_dev);
+ slave_port->rmnet_mode = RMNET_EPMODE_BRIDGE;
+ slave_port->bridge_ep = real_dev;
+
+ port->rmnet_mode = RMNET_EPMODE_BRIDGE;
+ port->bridge_ep = slave_dev;
+
+ netdev_dbg(slave_dev, "registered with rmnet as slave\n");
+ return 0;
+}
+
+int rmnet_del_bridge(struct net_device *rmnet_dev,
+ struct net_device *slave_dev)
+{
+ struct rmnet_priv *priv = netdev_priv(rmnet_dev);
+ struct net_device *real_dev = priv->real_dev;
+ struct rmnet_port *port, *slave_port;
+
+ port = rmnet_get_port(real_dev);
+ port->rmnet_mode = RMNET_EPMODE_VND;
+ port->bridge_ep = NULL;
+
+ netdev_upper_dev_unlink(slave_dev, rmnet_dev);
+ slave_port = rmnet_get_port(slave_dev);
+ rmnet_unregister_real_device(slave_dev, slave_port);
+
+ netdev_dbg(slave_dev, "removed from rmnet as slave\n");
+ return 0;
+}
+
/* Startup/Shutdown */
static int __init rmnet_init(void)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 8849986..60115e6 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -36,6 +36,7 @@ struct rmnet_port {
u8 nr_rmnet_devs;
u8 rmnet_mode;
struct hlist_head muxed_ep[RMNET_MAX_LOGICAL_EP];
+ struct net_device *bridge_ep;
};
extern struct rtnl_link_ops rmnet_link_ops;
@@ -47,5 +48,9 @@ struct rmnet_priv {
struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id);
-
+int rmnet_add_bridge(struct net_device *rmnet_dev,
+ struct net_device *slave_dev,
+ struct netlink_ext_ack *extack);
+int rmnet_del_bridge(struct net_device *rmnet_dev,
+ struct net_device *slave_dev);
#endif /* _RMNET_CONFIG_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index fa24ffb..02038e8 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -149,6 +149,17 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
return RMNET_MAP_SUCCESS;
}
+static rx_handler_result_t
+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 */
/* Processes packet as per ingress data format for receiving device. Logical
@@ -168,8 +179,15 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
dev = skb->dev;
port = rmnet_get_port(dev);
+ switch (port->rmnet_mode) {
+ case RMNET_EPMODE_VND:
if (port->ingress_data_format & RMNET_INGRESS_FORMAT_MAP)
rc = rmnet_map_ingress_handler(skb, port);
+ break;
+ case RMNET_EPMODE_BRIDGE:
+ rc = rmnet_bridge_handler(skb, port->bridge_ep);
+ break;
+ }
return rc;
}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 1b6747d..12bd0bb 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -74,6 +74,8 @@ static int rmnet_vnd_get_iflink(const struct net_device *dev)
.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,
};
/* Called by kernel whenever a new rmnet<n> device is created. Sets MTU,
--
1.9.1
^ permalink raw reply related
* [PATCH] r8169: only enable PCI wakeups when WOL is active
From: Daniel Drake @ 2017-10-11 4:56 UTC (permalink / raw)
To: nic_swsd; +Cc: netdev, romieu, linux
rtl_init_one() currently enables PCI wakeups if the ethernet device
is found to be WOL-capable. There is no need to do this when
rtl8169_set_wol() will correctly enable or disable the same wakeup flag
when WOL is activated/deactivated.
This works around an ACPI DSDT bug which prevents the Acer laptop models
Aspire ES1-533, Aspire ES1-732, PackardBell ENTE69AP and Gateway NE533
from entering S3 suspend - even when no ethernet cable is connected.
On these platforms, the DSDT says that GPE08 is a wakeup source for
ethernet, but this GPE fires as soon as the system goes into suspend,
waking the system up immediately. Having the wakeup normally disabled
avoids this issue in the default case.
With this change, WOL will continue to be unusable on these platforms
(it will instantly wake up if WOL is later enabled by the user) but we
do not expect this to be a commonly used feature on these consumer
laptops. We have separately determined that WOL works fine without any
ACPI GPEs enabled during sleep, so a DSDT fix or override would be
possible to make WOL work.
Signed-off-by: Daniel Drake <drake@endlessm.com>
---
drivers/net/ethernet/realtek/r8169.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index e03fcf914690..a3c949ea7d1a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8491,8 +8491,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
rtl8168_driver_start(tp);
}
- device_set_wakeup_enable(&pdev->dev, tp->features & RTL_FEATURE_WOL);
-
if (pci_dev_run_wake(pdev))
pm_runtime_put_noidle(&pdev->dev);
--
2.11.0
^ permalink raw reply related
* Re: [net-next V6 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: Jesper Dangaard Brouer @ 2017-10-11 5:36 UTC (permalink / raw)
To: Daniel Borkmann
Cc: netdev, jakub.kicinski, Michael S. Tsirkin, pavel.odintsov,
Jason Wang, mchan, John Fastabend, peter.waskiewicz.jr,
Daniel Borkmann, Alexei Starovoitov, Andy Gospodarek, brouer
In-Reply-To: <59DD4E48.3060403@iogearbox.net>
On Wed, 11 Oct 2017 00:48:40 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/10/2017 02:47 PM, Jesper Dangaard Brouer wrote:
> [...]
> > +static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
> > +{
> > + struct bpf_cpu_map *cmap;
> > + int err = -ENOMEM;
> > + u64 cost;
> > + int ret;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* check sanity of attributes */
> > + if (attr->max_entries == 0 || attr->key_size != 4 ||
> > + attr->value_size != 4 || attr->map_flags & ~BPF_F_NUMA_NODE)
> > + return ERR_PTR(-EINVAL);
> > +
> > + cmap = kzalloc(sizeof(*cmap), GFP_USER);
> > + if (!cmap)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + /* mandatory map attributes */
> > + cmap->map.map_type = attr->map_type;
> > + cmap->map.key_size = attr->key_size;
> > + cmap->map.value_size = attr->value_size;
> > + cmap->map.max_entries = attr->max_entries;
> > + cmap->map.map_flags = attr->map_flags;
> > + cmap->map.numa_node = bpf_map_attr_numa_node(attr);
> > +
> > + /* Pre-limit array size based on NR_CPUS, not final CPU check */
> > + if (cmap->map.max_entries > NR_CPUS)
> > + return ERR_PTR(-E2BIG);
>
> We still have a leak here, meaning kfree(cmap) is missing on above error.
Darn... yes, I introduced this in this V6 as I moved the check.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: High CPU load by native_queued_spin_lock_slowpath
From: Sergey K. @ 2017-10-11 5:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1507644432.31614.13.camel@edumazet-glaptop3.roam.corp.google.com>
I'm using ifb0 device for outgoing traffic.
I have one bond0 interface with exit to the Internet, and 2 interfaces
eth0 and eth2 to local users.
ifb0 - for shaping Internet traffic from bond0 to eth2 or eth0.
All outgoing traffic to the eth0 and eth2 redirecting to ifb0.
> What about multiple ifb instead, one per RX queue ?
You are offering to redirect traffic from every queue to personal ifb
device? I do not quite understand.
2017-10-10 20:07 GMT+06:00 Eric Dumazet <eric.dumazet@gmail.com>:
> On Tue, 2017-10-10 at 18:00 +0600, Sergey K. wrote:
>> I'm using Debian 9(stretch edition) kernel 4.9., hp dl385 g7 server
>> with 32 cpu cores. NIC queues are tied to processor cores. Server is
>> shaping traffic (iproute2 and htb discipline + skbinfo + ipset + ifb)
>> and filtering some rules by iptables.
>>
>> At that moment, when traffic goes up about 1gbit/s cpu is very high
>> loaded. Perf tool tells me that kernel module
>> native_queued_spin_lock_slowpath loading cpu about 40%.
>>
>> After several hours of searching, I found that if I remove the htb
>> discipline from ifb0, the high load goes down.
>> Well, I think that problem with classify and shaping by htb.
>>
>> Who knows how to solve?
>
> You use a single ifb0 on the whole (multiqueue) device for ingress ?
>
> What about multiple ifb instead, one per RX queue ?
>
> Alternative is to reduce contention and use a single RX queue.
>
>
^ permalink raw reply
* [PATCH net] net/ncsi: Don't limit vids based on hot_channel
From: Samuel Mendoza-Jonas @ 2017-10-11 5:54 UTC (permalink / raw)
To: netdev; +Cc: Samuel Mendoza-Jonas, David S . Miller, linux-kernel
Currently we drop any new VLAN ids if there are more than the current
(or last used) channel can support. Most importantly this is a problem
if no channel has been selected yet, resulting in a segfault.
Secondly this does not necessarily reflect the capabilities of any other
channels. Instead only drop a new VLAN id if we are already tracking the
maximum allowed by the NCSI specification. Per-channel limits are
already handled by ncsi_add_filter(), but add a message to set_one_vid()
to make it obvious that the channel can not support any more VLAN ids.
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 17 +++++++++--------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index af3d636534ef..d30f7bd741d0 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -286,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work; /* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_head node; /* Form NCSI device list */
+#define NCSI_MAX_VLAN_VIDS 15
struct list_head vlan_vids; /* List of active VLAN IDs */
};
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3fd3c39e6278..b6a449aa9d4b 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -732,6 +732,10 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
if (index < 0) {
netdev_err(ndp->ndev.dev,
"Failed to add new VLAN tag, error %d\n", index);
+ if (index == -ENOSPC)
+ netdev_err(ndp->ndev.dev,
+ "Channel %u already has all VLAN filters set\n",
+ nc->id);
return -1;
}
@@ -1403,7 +1407,6 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
{
- struct ncsi_channel_filter *ncf;
struct ncsi_dev_priv *ndp;
unsigned int n_vids = 0;
struct vlan_vid *vlan;
@@ -1420,7 +1423,6 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
}
ndp = TO_NCSI_DEV_PRIV(nd);
- ncf = ndp->hot_channel->filters[NCSI_FILTER_VLAN];
/* Add the VLAN id to our internal list */
list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
@@ -1431,12 +1433,11 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
return 0;
}
}
-
- if (n_vids >= ncf->total) {
- netdev_info(dev,
- "NCSI Channel supports up to %u VLAN tags but %u are already set\n",
- ncf->total, n_vids);
- return -EINVAL;
+ if (n_vids >= NCSI_MAX_VLAN_VIDS) {
+ netdev_warn(dev,
+ "tried to add vlan id %u but NCSI max already registered (%u)\n",
+ vid, NCSI_MAX_VLAN_VIDS);
+ return -ENOSPC;
}
vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
--
2.14.2
^ permalink raw reply related
* Re: [net-next V6 PATCH 0/5] New bpf cpumap type for XDP_REDIRECT
From: John Fastabend @ 2017-10-11 6:10 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev
Cc: jakub.kicinski, Michael S. Tsirkin, pavel.odintsov, Jason Wang,
mchan, peter.waskiewicz.jr, Daniel Borkmann, Alexei Starovoitov,
Andy Gospodarek
In-Reply-To: <150763962554.14394.15623435724195136364.stgit@firesoul>
On 10/10/2017 05:47 AM, Jesper Dangaard Brouer wrote:
> Introducing a new way to redirect XDP frames. Notice how no driver
> changes are necessary given the design of XDP_REDIRECT.
>
> This redirect map type is called 'cpumap', as it allows redirection
> XDP frames to remote CPUs. The remote CPU will do the SKB allocation
> and start the network stack invocation on that CPU.
>
> This is a scalability and isolation mechanism, that allow separating
> the early driver network XDP layer, from the rest of the netstack, and
> assigning dedicated CPUs for this stage. The sysadm control/configure
> the RX-CPU to NIC-RX queue (as usual) via procfs smp_affinity and how
> many queues are configured via ethtool --set-channels. Benchmarks
> show that a single CPU can handle approx 11Mpps. Thus, only assigning
> two NIC RX-queues (and two CPUs) is sufficient for handling 10Gbit/s
> wirespeed smallest packet 14.88Mpps. Reducing the number of queues
> have the advantage that more packets being "bulk" available per hard
> interrupt[1].
>
> [1] https://www.netdevconf.org/2.1/papers/BusyPollingNextGen.pdf
>
> Use-cases:
>
> 1. End-host based pre-filtering for DDoS mitigation. This is fast
> enough to allow software to see and filter all packets wirespeed.
> Thus, no packets getting silently dropped by hardware.
>
> 2. Given NIC HW unevenly distributes packets across RX queue, this
> mechanism can be used for redistribution load across CPUs. This
> usually happens when HW is unaware of a new protocol. This
> resembles RPS (Receive Packet Steering), just faster, but with more
> responsibility placed on the BPF program for correct steering.
Hi Jesper,
Another (somewhat meta) comment about the performance benchmarks. In
one of the original threads you showed that the XDP cpu map outperformed
RPS in TCP_CRR netperf tests. It was significant iirc in the mpps range.
But, with this series we will skip GRO. Do you have any idea how this
looks with other tests such as TCP_STREAM? I'm trying to understand
if this is something that can be used in the general case or is more
for the special case and will have to be enabled/disabled by the
orchestration layer depending on workload/network conditions.
My intuition is the general case will be slower due to lack of GRO. If
this is the case any ideas how we could add GRO? Not needed in the
initial patchset but trying to see if the two are mutually exclusive. I
don't off-hand see an easy way to pull GRO into this feature.
Thanks,
John
^ permalink raw reply
* Re: [PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN
From: Herbert Xu @ 2017-10-11 6:26 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Mike Snitzer, linux-doc, Gary Hook, David Howells,
device-mapper development, keyrings, linux-ima-devel,
Alasdair Kergon, Steffen Klassert, Boris Brezillon,
Jonathan Corbet, Alexey Kuznetsov, Mimi Zohar, Serge E. Hallyn,
Tom Lendacky, linux-cifs, linux-ima-user, Arnaud Ebalard,
linux-raid, linux-fscrypt, linux-mediatek, James Morris,
Matthias Brugger, Jaegeuk Kim
In-Reply-To: <CAOtvUMd=vtju=VJkCYFRrvANbRRYj+95cdgDoMMfBvmPaPbsAA@mail.gmail.com>
On Sat, Oct 07, 2017 at 10:51:42AM +0300, Gilad Ben-Yossef wrote:
> On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote:
> >>
> >> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> >> index 5e92bd2..3b3c154 100644
> >> --- a/crypto/algif_hash.c
> >> +++ b/crypto/algif_hash.c
> >> @@ -39,6 +39,20 @@ struct algif_hash_tfm {
> >> bool has_key;
> >> };
> >>
> >> +/* Previous versions of crypto_* ops used to return -EBUSY
> >> + * rather than -EAGAIN to indicate being tied up. The in
> >> + * kernel API changed but we don't want to break the user
> >> + * space API. As only the hash user interface exposed this
> >> + * error ever to the user, do the translation here.
> >> + */
> >> +static inline int crypto_user_err(int err)
> >> +{
> >> + if (err == -EAGAIN)
> >> + return -EBUSY;
> >> +
> >> + return err;
> >
> > I don't see the need to carry along this baggage. Does anyone
> > in user-space actually rely on EBUSY?
>
>
> I am not aware of anyone who does. I was just trying to avoid
> changing the user ABI.
>
> Shall I roll a new revision without this patch?
Yes please. I'd rather not carry this around for eternity unless
it was actually required.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [patch net-next 0/4] net: sched: get rid of cls_flower->egress_dev
From: Jiri Pirko @ 2017-10-11 6:44 UTC (permalink / raw)
To: Or Gerlitz
Cc: Saeed Mahameed, Linux Netdev List, David Miller, Jamal Hadi Salim,
Cong Wang, mlxsw
In-Reply-To: <CAJ3xEMizUNPzPAjWuk-PpkK88UgD9OmOGORiEO64r0dx-GfcBw@mail.gmail.com>
Tue, Oct 10, 2017 at 11:46:22PM CEST, gerlitz.or@gmail.com wrote:
>On Wed, Oct 11, 2017 at 12:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Tue, Oct 10, 2017 at 07:24:21PM CEST, gerlitz.or@gmail.com wrote:
>
>> Or, as I replied to you earlier, the issue you describe is totally
>> unrelated to this patchset as you see the issue with the current net-next.
>
>Jiri, the point I wanted to make that if indeed there's a bug in mlx5
>or flower, we will have to fix it for 4.14 and then these bits would
>have to be rebased when net-next is re-planted over net, I put "FWIW"
>before that, so maybe it doesn't W so much, we'll see.
The fix is still unrelated to this patchset.
^ permalink raw reply
* [PATCH v2 0/7] net: qrtr: Fixes and support receiving version 2 packets
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
On the latest Qualcomm platforms remote processors are sending packets with
version 2 of the message header. This series starts off with some fixes and
then refactors the qrtr code to support receiving messages of both version 1
and version 2.
As all remotes are backwards compatible transmitted packets continues to be
send as version 1, but some groundwork has been done to make this a per-link
property.
Bjorn Andersson (7):
net: qrtr: Invoke sk_error_report() after setting sk_err
net: qrtr: Move constants to header file
net: qrtr: Add control packet definition to uapi
net: qrtr: Pass source and destination to enqueue functions
net: qrtr: Clean up control packet handling
net: qrtr: Use sk_buff->cb in receive path
net: qrtr: Support decoding incoming v2 packets
include/uapi/linux/qrtr.h | 35 +++++
net/qrtr/qrtr.c | 377 +++++++++++++++++++++++++---------------------
2 files changed, 241 insertions(+), 171 deletions(-)
--
2.12.0
^ permalink raw reply
* [PATCH v2 1/7] net: qrtr: Invoke sk_error_report() after setting sk_err
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
Rather than manually waking up any context sleeping on the sock to
signal an error we should call sk_error_report(). This has the added
benefit that in-kernel consumers can override this notification with
its own callback.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
net/qrtr/qrtr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index c2f5c13550c0..7e4b49a8349e 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -541,7 +541,7 @@ static void qrtr_reset_ports(void)
sock_hold(&ipc->sk);
ipc->sk.sk_err = ENETRESET;
- wake_up_interruptible(sk_sleep(&ipc->sk));
+ ipc->sk.sk_error_report(&ipc->sk);
sock_put(&ipc->sk);
}
mutex_unlock(&qrtr_port_lock);
--
2.12.0
^ permalink raw reply related
* [PATCH v2 2/7] net: qrtr: Move constants to header file
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
The constants are used by both the name server and clients, so clarify
their value and move them to the uapi header.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
include/uapi/linux/qrtr.h | 3 +++
net/qrtr/qrtr.c | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 9d76c566f66e..63e8803e4d90 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -4,6 +4,9 @@
#include <linux/socket.h>
#include <linux/types.h>
+#define QRTR_NODE_BCAST 0xffffffffu
+#define QRTR_PORT_CTRL 0xfffffffeu
+
struct sockaddr_qrtr {
__kernel_sa_family_t sq_family;
__u32 sq_node;
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 7e4b49a8349e..15981abc042c 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -61,8 +61,6 @@ struct qrtr_hdr {
} __packed;
#define QRTR_HDR_SIZE sizeof(struct qrtr_hdr)
-#define QRTR_NODE_BCAST ((unsigned int)-1)
-#define QRTR_PORT_CTRL ((unsigned int)-2)
struct qrtr_sock {
/* WARNING: sk must be the first member */
--
2.12.0
^ permalink raw reply related
* [PATCH v2 3/7] net: qrtr: Add control packet definition to uapi
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
The QMUX protocol specification defines structure of the special control
packet messages being sent between handlers of the control port.
Add these to the uapi header, as this structure and the associated types
are shared between the kernel and all userspace handlers of control
messages.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
include/uapi/linux/qrtr.h | 32 ++++++++++++++++++++++++++++++++
net/qrtr/qrtr.c | 12 ------------
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 63e8803e4d90..179af64846e0 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -13,4 +13,36 @@ struct sockaddr_qrtr {
__u32 sq_port;
};
+enum qrtr_pkt_type {
+ QRTR_TYPE_DATA = 1,
+ QRTR_TYPE_HELLO = 2,
+ QRTR_TYPE_BYE = 3,
+ QRTR_TYPE_NEW_SERVER = 4,
+ QRTR_TYPE_DEL_SERVER = 5,
+ QRTR_TYPE_DEL_CLIENT = 6,
+ QRTR_TYPE_RESUME_TX = 7,
+ QRTR_TYPE_EXIT = 8,
+ QRTR_TYPE_PING = 9,
+ QRTR_TYPE_NEW_LOOKUP = 10,
+ QRTR_TYPE_DEL_LOOKUP = 11,
+};
+
+struct qrtr_ctrl_pkt {
+ __le32 cmd;
+
+ union {
+ struct {
+ __le32 service;
+ __le32 instance;
+ __le32 node;
+ __le32 port;
+ } server;
+
+ struct {
+ __le32 node;
+ __le32 port;
+ } client;
+ };
+} __packed;
+
#endif /* _LINUX_QRTR_H */
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 15981abc042c..d85ca7170b8f 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -26,18 +26,6 @@
#define QRTR_MIN_EPH_SOCKET 0x4000
#define QRTR_MAX_EPH_SOCKET 0x7fff
-enum qrtr_pkt_type {
- QRTR_TYPE_DATA = 1,
- QRTR_TYPE_HELLO = 2,
- QRTR_TYPE_BYE = 3,
- QRTR_TYPE_NEW_SERVER = 4,
- QRTR_TYPE_DEL_SERVER = 5,
- QRTR_TYPE_DEL_CLIENT = 6,
- QRTR_TYPE_RESUME_TX = 7,
- QRTR_TYPE_EXIT = 8,
- QRTR_TYPE_PING = 9,
-};
-
/**
* struct qrtr_hdr - (I|R)PCrouter packet header
* @version: protocol version
--
2.12.0
^ permalink raw reply related
* [PATCH v2 4/7] net: qrtr: Pass source and destination to enqueue functions
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
Defer writing the message header to the skb until its time to enqueue
the packet. As the receive path is reworked to decode the message header
as it's received from the transport and only pass around the payload in
the skb this change means that we do not have to fill out the full
message header just to decode it immediately in qrtr_local_enqueue().
In the future this change also makes it possible to prepend message
headers based on the version of each link.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
net/qrtr/qrtr.c | 120 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 69 insertions(+), 51 deletions(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index d85ca7170b8f..82dc83789310 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -97,8 +97,12 @@ struct qrtr_node {
struct list_head item;
};
-static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb);
-static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb);
+static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
+ int type, struct sockaddr_qrtr *from,
+ struct sockaddr_qrtr *to);
+static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb,
+ int type, struct sockaddr_qrtr *from,
+ struct sockaddr_qrtr *to);
/* Release node resources and free the node.
*
@@ -136,10 +140,27 @@ static void qrtr_node_release(struct qrtr_node *node)
}
/* Pass an outgoing packet socket buffer to the endpoint driver. */
-static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb)
+static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
+ int type, struct sockaddr_qrtr *from,
+ struct sockaddr_qrtr *to)
{
+ struct qrtr_hdr *hdr;
+ size_t len = skb->len;
int rc = -ENODEV;
+ hdr = skb_push(skb, QRTR_HDR_SIZE);
+ hdr->version = cpu_to_le32(QRTR_PROTO_VER);
+ hdr->type = cpu_to_le32(type);
+ hdr->src_node_id = cpu_to_le32(from->sq_node);
+ hdr->src_port_id = cpu_to_le32(from->sq_port);
+ hdr->dst_node_id = cpu_to_le32(to->sq_node);
+ hdr->dst_port_id = cpu_to_le32(to->sq_port);
+
+ hdr->size = cpu_to_le32(len);
+ hdr->confirm_rx = 0;
+
+ skb_put_padto(skb, ALIGN(len, 4));
+
mutex_lock(&node->ep_lock);
if (node->ep)
rc = node->ep->xmit(node->ep, skb);
@@ -237,23 +258,13 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
static struct sk_buff *qrtr_alloc_ctrl_packet(u32 type, size_t pkt_len,
u32 src_node, u32 dst_node)
{
- struct qrtr_hdr *hdr;
struct sk_buff *skb;
skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL);
if (!skb)
return NULL;
- skb_reset_transport_header(skb);
- hdr = skb_put(skb, QRTR_HDR_SIZE);
- hdr->version = cpu_to_le32(QRTR_PROTO_VER);
- hdr->type = cpu_to_le32(type);
- hdr->src_node_id = cpu_to_le32(src_node);
- hdr->src_port_id = cpu_to_le32(QRTR_PORT_CTRL);
- hdr->confirm_rx = cpu_to_le32(0);
- hdr->size = cpu_to_le32(pkt_len);
- hdr->dst_node_id = cpu_to_le32(dst_node);
- hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL);
+ skb_reserve(skb, QRTR_HDR_SIZE);
return skb;
}
@@ -326,6 +337,8 @@ static void qrtr_port_put(struct qrtr_sock *ipc);
static void qrtr_node_rx_work(struct work_struct *work)
{
struct qrtr_node *node = container_of(work, struct qrtr_node, work);
+ struct sockaddr_qrtr dst;
+ struct sockaddr_qrtr src;
struct sk_buff *skb;
while ((skb = skb_dequeue(&node->rx_queue)) != NULL) {
@@ -341,6 +354,11 @@ static void qrtr_node_rx_work(struct work_struct *work)
dst_port = le32_to_cpu(phdr->dst_port_id);
confirm = !!phdr->confirm_rx;
+ src.sq_node = src_node;
+ src.sq_port = le32_to_cpu(phdr->src_port_id);
+ dst.sq_node = dst_node;
+ dst.sq_port = dst_port;
+
qrtr_node_assign(node, src_node);
ipc = qrtr_port_lookup(dst_port);
@@ -357,7 +375,9 @@ static void qrtr_node_rx_work(struct work_struct *work)
skb = qrtr_alloc_resume_tx(dst_node, node->nid, dst_port);
if (!skb)
break;
- if (qrtr_node_enqueue(node, skb))
+
+ if (qrtr_node_enqueue(node, skb, QRTR_TYPE_RESUME_TX,
+ &dst, &src))
break;
}
}
@@ -407,6 +427,8 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_register);
void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
{
struct qrtr_node *node = ep->node;
+ struct sockaddr_qrtr src = {AF_QIPCRTR, node->nid, QRTR_PORT_CTRL};
+ struct sockaddr_qrtr dst = {AF_QIPCRTR, qrtr_local_nid, QRTR_PORT_CTRL};
struct sk_buff *skb;
mutex_lock(&node->ep_lock);
@@ -416,7 +438,7 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
/* Notify the local controller about the event */
skb = qrtr_alloc_local_bye(node->nid);
if (skb)
- qrtr_local_enqueue(NULL, skb);
+ qrtr_local_enqueue(NULL, skb, QRTR_TYPE_BYE, &src, &dst);
qrtr_node_release(node);
ep->node = NULL;
@@ -454,11 +476,17 @@ static void qrtr_port_remove(struct qrtr_sock *ipc)
{
struct sk_buff *skb;
int port = ipc->us.sq_port;
+ struct sockaddr_qrtr to;
+
+ to.sq_family = AF_QIPCRTR;
+ to.sq_node = QRTR_NODE_BCAST;
+ to.sq_port = QRTR_PORT_CTRL;
skb = qrtr_alloc_del_client(&ipc->us);
if (skb) {
skb_set_owner_w(skb, &ipc->sk);
- qrtr_bcast_enqueue(NULL, skb);
+ qrtr_bcast_enqueue(NULL, skb, QRTR_TYPE_DEL_CLIENT, &ipc->us,
+ &to);
}
if (port == QRTR_PORT_CTRL)
@@ -606,19 +634,25 @@ static int qrtr_bind(struct socket *sock, struct sockaddr *saddr, int len)
}
/* Queue packet to local peer socket. */
-static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb)
+static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
+ int type, struct sockaddr_qrtr *from,
+ struct sockaddr_qrtr *to)
{
- const struct qrtr_hdr *phdr;
struct qrtr_sock *ipc;
+ struct qrtr_hdr *phdr;
- phdr = (const struct qrtr_hdr *)skb_transport_header(skb);
-
- ipc = qrtr_port_lookup(le32_to_cpu(phdr->dst_port_id));
+ ipc = qrtr_port_lookup(to->sq_port);
if (!ipc || &ipc->sk == skb->sk) { /* do not send to self */
kfree_skb(skb);
return -ENODEV;
}
+ phdr = skb_push(skb, QRTR_HDR_SIZE);
+ skb_reset_transport_header(skb);
+
+ phdr->src_node_id = cpu_to_le32(from->sq_node);
+ phdr->src_port_id = cpu_to_le32(from->sq_port);
+
if (sock_queue_rcv_skb(&ipc->sk, skb)) {
qrtr_port_put(ipc);
kfree_skb(skb);
@@ -631,7 +665,9 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb)
}
/* Queue packet for broadcast. */
-static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb)
+static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb,
+ int type, struct sockaddr_qrtr *from,
+ struct sockaddr_qrtr *to)
{
struct sk_buff *skbn;
@@ -641,11 +677,11 @@ static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb)
if (!skbn)
break;
skb_set_owner_w(skbn, skb->sk);
- qrtr_node_enqueue(node, skbn);
+ qrtr_node_enqueue(node, skbn, type, from, to);
}
mutex_unlock(&qrtr_node_lock);
- qrtr_local_enqueue(node, skb);
+ qrtr_local_enqueue(node, skb, type, from, to);
return 0;
}
@@ -653,13 +689,14 @@ static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb)
static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
{
DECLARE_SOCKADDR(struct sockaddr_qrtr *, addr, msg->msg_name);
- int (*enqueue_fn)(struct qrtr_node *, struct sk_buff *);
+ int (*enqueue_fn)(struct qrtr_node *, struct sk_buff *, int,
+ struct sockaddr_qrtr *, struct sockaddr_qrtr *);
struct qrtr_sock *ipc = qrtr_sk(sock->sk);
struct sock *sk = sock->sk;
struct qrtr_node *node;
- struct qrtr_hdr *hdr;
struct sk_buff *skb;
size_t plen;
+ u32 type = QRTR_TYPE_DATA;
int rc;
if (msg->msg_flags & ~(MSG_DONTWAIT))
@@ -713,32 +750,14 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
if (!skb)
goto out_node;
- skb_reset_transport_header(skb);
- skb_put(skb, len + QRTR_HDR_SIZE);
-
- hdr = (struct qrtr_hdr *)skb_transport_header(skb);
- hdr->version = cpu_to_le32(QRTR_PROTO_VER);
- hdr->src_node_id = cpu_to_le32(ipc->us.sq_node);
- hdr->src_port_id = cpu_to_le32(ipc->us.sq_port);
- hdr->confirm_rx = cpu_to_le32(0);
- hdr->size = cpu_to_le32(len);
- hdr->dst_node_id = cpu_to_le32(addr->sq_node);
- hdr->dst_port_id = cpu_to_le32(addr->sq_port);
+ skb_reserve(skb, QRTR_HDR_SIZE);
- rc = skb_copy_datagram_from_iter(skb, QRTR_HDR_SIZE,
- &msg->msg_iter, len);
+ rc = memcpy_from_msg(skb_put(skb, len), msg, len);
if (rc) {
kfree_skb(skb);
goto out_node;
}
- if (plen != len) {
- rc = skb_pad(skb, plen - len);
- if (rc)
- goto out_node;
- skb_put(skb, plen - len);
- }
-
if (ipc->us.sq_port == QRTR_PORT_CTRL) {
if (len < 4) {
rc = -EINVAL;
@@ -747,12 +766,11 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
}
/* control messages already require the type as 'command' */
- skb_copy_bits(skb, QRTR_HDR_SIZE, &hdr->type, 4);
- } else {
- hdr->type = cpu_to_le32(QRTR_TYPE_DATA);
+ skb_copy_bits(skb, 0, &type, 4);
+ type = le32_to_cpu(type);
}
- rc = enqueue_fn(node, skb);
+ rc = enqueue_fn(node, skb, type, &ipc->us, addr);
if (rc >= 0)
rc = len;
--
2.12.0
^ permalink raw reply related
* [PATCH v2 5/7] net: qrtr: Clean up control packet handling
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
As the message header generation is deferred the internal functions for
generating control packets can be simplified.
This patch modifies qrtr_alloc_ctrl_packet() to, in addition to the
sk_buff, return a reference to a struct qrtr_ctrl_pkt, which clarifies
and simplifies the helpers to the point that these functions can be
folded back into the callers.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
net/qrtr/qrtr.c | 93 ++++++++++++++++++---------------------------------------
1 file changed, 29 insertions(+), 64 deletions(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 82dc83789310..a84edba7b1ef 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -255,9 +255,18 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
}
EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
-static struct sk_buff *qrtr_alloc_ctrl_packet(u32 type, size_t pkt_len,
- u32 src_node, u32 dst_node)
+/**
+ * qrtr_alloc_ctrl_packet() - allocate control packet skb
+ * @pkt: reference to qrtr_ctrl_pkt pointer
+ *
+ * Returns newly allocated sk_buff, or NULL on failure
+ *
+ * This function allocates a sk_buff large enough to carry a qrtr_ctrl_pkt and
+ * on success returns a reference to the control packet in @pkt.
+ */
+static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt)
{
+ const int pkt_len = sizeof(struct qrtr_ctrl_pkt);
struct sk_buff *skb;
skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL);
@@ -265,64 +274,7 @@ static struct sk_buff *qrtr_alloc_ctrl_packet(u32 type, size_t pkt_len,
return NULL;
skb_reserve(skb, QRTR_HDR_SIZE);
-
- return skb;
-}
-
-/* Allocate and construct a resume-tx packet. */
-static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
- u32 dst_node, u32 port)
-{
- const int pkt_len = 20;
- struct sk_buff *skb;
- __le32 *buf;
-
- skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_RESUME_TX, pkt_len,
- src_node, dst_node);
- if (!skb)
- return NULL;
-
- buf = skb_put_zero(skb, pkt_len);
- buf[0] = cpu_to_le32(QRTR_TYPE_RESUME_TX);
- buf[1] = cpu_to_le32(src_node);
- buf[2] = cpu_to_le32(port);
-
- return skb;
-}
-
-/* Allocate and construct a BYE message to signal remote termination */
-static struct sk_buff *qrtr_alloc_local_bye(u32 src_node)
-{
- const int pkt_len = 20;
- struct sk_buff *skb;
- __le32 *buf;
-
- skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_BYE, pkt_len,
- src_node, qrtr_local_nid);
- if (!skb)
- return NULL;
-
- buf = skb_put_zero(skb, pkt_len);
- buf[0] = cpu_to_le32(QRTR_TYPE_BYE);
-
- return skb;
-}
-
-static struct sk_buff *qrtr_alloc_del_client(struct sockaddr_qrtr *sq)
-{
- const int pkt_len = 20;
- struct sk_buff *skb;
- __le32 *buf;
-
- skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_DEL_CLIENT, pkt_len,
- sq->sq_node, QRTR_NODE_BCAST);
- if (!skb)
- return NULL;
-
- buf = skb_put_zero(skb, pkt_len);
- buf[0] = cpu_to_le32(QRTR_TYPE_DEL_CLIENT);
- buf[1] = cpu_to_le32(sq->sq_node);
- buf[2] = cpu_to_le32(sq->sq_port);
+ *pkt = skb_put_zero(skb, pkt_len);
return skb;
}
@@ -337,6 +289,7 @@ static void qrtr_port_put(struct qrtr_sock *ipc);
static void qrtr_node_rx_work(struct work_struct *work)
{
struct qrtr_node *node = container_of(work, struct qrtr_node, work);
+ struct qrtr_ctrl_pkt *pkt;
struct sockaddr_qrtr dst;
struct sockaddr_qrtr src;
struct sk_buff *skb;
@@ -372,10 +325,14 @@ static void qrtr_node_rx_work(struct work_struct *work)
}
if (confirm) {
- skb = qrtr_alloc_resume_tx(dst_node, node->nid, dst_port);
+ skb = qrtr_alloc_ctrl_packet(&pkt);
if (!skb)
break;
+ pkt->cmd = cpu_to_le32(QRTR_TYPE_RESUME_TX);
+ pkt->client.node = cpu_to_le32(dst.sq_node);
+ pkt->client.port = cpu_to_le32(dst.sq_port);
+
if (qrtr_node_enqueue(node, skb, QRTR_TYPE_RESUME_TX,
&dst, &src))
break;
@@ -429,6 +386,7 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
struct qrtr_node *node = ep->node;
struct sockaddr_qrtr src = {AF_QIPCRTR, node->nid, QRTR_PORT_CTRL};
struct sockaddr_qrtr dst = {AF_QIPCRTR, qrtr_local_nid, QRTR_PORT_CTRL};
+ struct qrtr_ctrl_pkt *pkt;
struct sk_buff *skb;
mutex_lock(&node->ep_lock);
@@ -436,9 +394,11 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
mutex_unlock(&node->ep_lock);
/* Notify the local controller about the event */
- skb = qrtr_alloc_local_bye(node->nid);
- if (skb)
+ skb = qrtr_alloc_ctrl_packet(&pkt);
+ if (skb) {
+ pkt->cmd = cpu_to_le32(QRTR_TYPE_BYE);
qrtr_local_enqueue(NULL, skb, QRTR_TYPE_BYE, &src, &dst);
+ }
qrtr_node_release(node);
ep->node = NULL;
@@ -474,6 +434,7 @@ static void qrtr_port_put(struct qrtr_sock *ipc)
/* Remove port assignment. */
static void qrtr_port_remove(struct qrtr_sock *ipc)
{
+ struct qrtr_ctrl_pkt *pkt;
struct sk_buff *skb;
int port = ipc->us.sq_port;
struct sockaddr_qrtr to;
@@ -482,8 +443,12 @@ static void qrtr_port_remove(struct qrtr_sock *ipc)
to.sq_node = QRTR_NODE_BCAST;
to.sq_port = QRTR_PORT_CTRL;
- skb = qrtr_alloc_del_client(&ipc->us);
+ skb = qrtr_alloc_ctrl_packet(&pkt);
if (skb) {
+ pkt->cmd = cpu_to_le32(QRTR_TYPE_DEL_CLIENT);
+ pkt->client.node = cpu_to_le32(ipc->us.sq_node);
+ pkt->client.port = cpu_to_le32(ipc->us.sq_port);
+
skb_set_owner_w(skb, &ipc->sk);
qrtr_bcast_enqueue(NULL, skb, QRTR_TYPE_DEL_CLIENT, &ipc->us,
&to);
--
2.12.0
^ permalink raw reply related
* [PATCH v2 6/7] net: qrtr: Use sk_buff->cb in receive path
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
Rather than parsing the header of incoming messages throughout the
implementation do it once when we retrieve the message and store the
relevant information in the "cb" member of the sk_buff.
This allows us to, in a later commit, decode version 2 messages into
this same structure.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- None
net/qrtr/qrtr.c | 70 ++++++++++++++++++++++++++++++++-------------------------
1 file changed, 40 insertions(+), 30 deletions(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index a84edba7b1ef..7bca6ec892a5 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -48,6 +48,16 @@ struct qrtr_hdr {
__le32 dst_port_id;
} __packed;
+struct qrtr_cb {
+ u32 src_node;
+ u32 src_port;
+ u32 dst_node;
+ u32 dst_port;
+
+ u8 type;
+ u8 confirm_rx;
+};
+
#define QRTR_HDR_SIZE sizeof(struct qrtr_hdr)
struct qrtr_sock {
@@ -216,6 +226,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
struct qrtr_node *node = ep->node;
const struct qrtr_hdr *phdr = data;
struct sk_buff *skb;
+ struct qrtr_cb *cb;
unsigned int psize;
unsigned int size;
unsigned int type;
@@ -245,8 +256,15 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
if (!skb)
return -ENOMEM;
- skb_reset_transport_header(skb);
- skb_put_data(skb, data, len);
+ cb = (struct qrtr_cb *)skb->cb;
+ cb->src_node = le32_to_cpu(phdr->src_node_id);
+ cb->src_port = le32_to_cpu(phdr->src_port_id);
+ cb->dst_node = le32_to_cpu(phdr->dst_node_id);
+ cb->dst_port = le32_to_cpu(phdr->dst_port_id);
+ cb->type = type;
+ cb->confirm_rx = !!phdr->confirm_rx;
+
+ skb_put_data(skb, data + QRTR_HDR_SIZE, size);
skb_queue_tail(&node->rx_queue, skb);
schedule_work(&node->work);
@@ -295,26 +313,20 @@ static void qrtr_node_rx_work(struct work_struct *work)
struct sk_buff *skb;
while ((skb = skb_dequeue(&node->rx_queue)) != NULL) {
- const struct qrtr_hdr *phdr;
- u32 dst_node, dst_port;
struct qrtr_sock *ipc;
- u32 src_node;
+ struct qrtr_cb *cb;
int confirm;
- phdr = (const struct qrtr_hdr *)skb_transport_header(skb);
- src_node = le32_to_cpu(phdr->src_node_id);
- dst_node = le32_to_cpu(phdr->dst_node_id);
- dst_port = le32_to_cpu(phdr->dst_port_id);
- confirm = !!phdr->confirm_rx;
+ cb = (struct qrtr_cb *)skb->cb;
+ src.sq_node = cb->src_node;
+ src.sq_port = cb->src_port;
+ dst.sq_node = cb->dst_node;
+ dst.sq_port = cb->dst_port;
+ confirm = !!cb->confirm_rx;
- src.sq_node = src_node;
- src.sq_port = le32_to_cpu(phdr->src_port_id);
- dst.sq_node = dst_node;
- dst.sq_port = dst_port;
+ qrtr_node_assign(node, cb->src_node);
- qrtr_node_assign(node, src_node);
-
- ipc = qrtr_port_lookup(dst_port);
+ ipc = qrtr_port_lookup(cb->dst_port);
if (!ipc) {
kfree_skb(skb);
} else {
@@ -604,7 +616,7 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
struct sockaddr_qrtr *to)
{
struct qrtr_sock *ipc;
- struct qrtr_hdr *phdr;
+ struct qrtr_cb *cb;
ipc = qrtr_port_lookup(to->sq_port);
if (!ipc || &ipc->sk == skb->sk) { /* do not send to self */
@@ -612,11 +624,9 @@ static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb,
return -ENODEV;
}
- phdr = skb_push(skb, QRTR_HDR_SIZE);
- skb_reset_transport_header(skb);
-
- phdr->src_node_id = cpu_to_le32(from->sq_node);
- phdr->src_port_id = cpu_to_le32(from->sq_port);
+ cb = (struct qrtr_cb *)skb->cb;
+ cb->src_node = from->sq_node;
+ cb->src_port = from->sq_port;
if (sock_queue_rcv_skb(&ipc->sk, skb)) {
qrtr_port_put(ipc);
@@ -750,9 +760,9 @@ static int qrtr_recvmsg(struct socket *sock, struct msghdr *msg,
size_t size, int flags)
{
DECLARE_SOCKADDR(struct sockaddr_qrtr *, addr, msg->msg_name);
- const struct qrtr_hdr *phdr;
struct sock *sk = sock->sk;
struct sk_buff *skb;
+ struct qrtr_cb *cb;
int copied, rc;
lock_sock(sk);
@@ -769,22 +779,22 @@ static int qrtr_recvmsg(struct socket *sock, struct msghdr *msg,
return rc;
}
- phdr = (const struct qrtr_hdr *)skb_transport_header(skb);
- copied = le32_to_cpu(phdr->size);
+ copied = skb->len;
if (copied > size) {
copied = size;
msg->msg_flags |= MSG_TRUNC;
}
- rc = skb_copy_datagram_msg(skb, QRTR_HDR_SIZE, msg, copied);
+ rc = skb_copy_datagram_msg(skb, 0, msg, copied);
if (rc < 0)
goto out;
rc = copied;
if (addr) {
+ cb = (struct qrtr_cb *)skb->cb;
addr->sq_family = AF_QIPCRTR;
- addr->sq_node = le32_to_cpu(phdr->src_node_id);
- addr->sq_port = le32_to_cpu(phdr->src_port_id);
+ addr->sq_node = cb->src_node;
+ addr->sq_port = cb->src_port;
msg->msg_namelen = sizeof(*addr);
}
@@ -877,7 +887,7 @@ static int qrtr_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
case TIOCINQ:
skb = skb_peek(&sk->sk_receive_queue);
if (skb)
- len = skb->len - QRTR_HDR_SIZE;
+ len = skb->len;
rc = put_user(len, (int __user *)argp);
break;
case SIOCGIFADDR:
--
2.12.0
^ permalink raw reply related
* [PATCH v2 7/7] net: qrtr: Support decoding incoming v2 packets
From: Bjorn Andersson @ 2017-10-11 6:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, linux-arm-msm, Chris Lew
In-Reply-To: <20171011064523.7902-1-bjorn.andersson@linaro.org>
Add the necessary logic for decoding incoming messages of version 2 as
well. Also make sure there's room for the bigger of version 1 and 2
headers in the code allocating skbs for outgoing messages.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- Dropped __packed from struct qrtr_hdr_v2
net/qrtr/qrtr.c | 132 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 94 insertions(+), 38 deletions(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 7bca6ec892a5..e458ece96d3d 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -20,14 +20,15 @@
#include "qrtr.h"
-#define QRTR_PROTO_VER 1
+#define QRTR_PROTO_VER_1 1
+#define QRTR_PROTO_VER_2 3
/* auto-bind range */
#define QRTR_MIN_EPH_SOCKET 0x4000
#define QRTR_MAX_EPH_SOCKET 0x7fff
/**
- * struct qrtr_hdr - (I|R)PCrouter packet header
+ * struct qrtr_hdr_v1 - (I|R)PCrouter packet header version 1
* @version: protocol version
* @type: packet type; one of QRTR_TYPE_*
* @src_node_id: source node
@@ -37,7 +38,7 @@
* @dst_node_id: destination node
* @dst_port_id: destination port
*/
-struct qrtr_hdr {
+struct qrtr_hdr_v1 {
__le32 version;
__le32 type;
__le32 src_node_id;
@@ -48,6 +49,32 @@ struct qrtr_hdr {
__le32 dst_port_id;
} __packed;
+/**
+ * struct qrtr_hdr_v2 - (I|R)PCrouter packet header later versions
+ * @version: protocol version
+ * @type: packet type; one of QRTR_TYPE_*
+ * @flags: bitmask of QRTR_FLAGS_*
+ * @optlen: length of optional header data
+ * @size: length of packet, excluding this header and optlen
+ * @src_node_id: source node
+ * @src_port_id: source port
+ * @dst_node_id: destination node
+ * @dst_port_id: destination port
+ */
+struct qrtr_hdr_v2 {
+ u8 version;
+ u8 type;
+ u8 flags;
+ u8 optlen;
+ __le32 size;
+ __le16 src_node_id;
+ __le16 src_port_id;
+ __le16 dst_node_id;
+ __le16 dst_port_id;
+};
+
+#define QRTR_FLAGS_CONFIRM_RX BIT(0)
+
struct qrtr_cb {
u32 src_node;
u32 src_port;
@@ -58,7 +85,8 @@ struct qrtr_cb {
u8 confirm_rx;
};
-#define QRTR_HDR_SIZE sizeof(struct qrtr_hdr)
+#define QRTR_HDR_MAX_SIZE max_t(size_t, sizeof(struct qrtr_hdr_v1), \
+ sizeof(struct qrtr_hdr_v2))
struct qrtr_sock {
/* WARNING: sk must be the first member */
@@ -154,12 +182,12 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
int type, struct sockaddr_qrtr *from,
struct sockaddr_qrtr *to)
{
- struct qrtr_hdr *hdr;
+ struct qrtr_hdr_v1 *hdr;
size_t len = skb->len;
int rc = -ENODEV;
- hdr = skb_push(skb, QRTR_HDR_SIZE);
- hdr->version = cpu_to_le32(QRTR_PROTO_VER);
+ hdr = skb_push(skb, sizeof(*hdr));
+ hdr->version = cpu_to_le32(QRTR_PROTO_VER_1);
hdr->type = cpu_to_le32(type);
hdr->src_node_id = cpu_to_le32(from->sq_node);
hdr->src_port_id = cpu_to_le32(from->sq_port);
@@ -224,52 +252,80 @@ static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid)
int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
{
struct qrtr_node *node = ep->node;
- const struct qrtr_hdr *phdr = data;
+ const struct qrtr_hdr_v1 *v1;
+ const struct qrtr_hdr_v2 *v2;
struct sk_buff *skb;
struct qrtr_cb *cb;
- unsigned int psize;
unsigned int size;
- unsigned int type;
unsigned int ver;
- unsigned int dst;
+ size_t hdrlen;
- if (len < QRTR_HDR_SIZE || len & 3)
+ if (len & 3)
return -EINVAL;
- ver = le32_to_cpu(phdr->version);
- size = le32_to_cpu(phdr->size);
- type = le32_to_cpu(phdr->type);
- dst = le32_to_cpu(phdr->dst_port_id);
+ skb = netdev_alloc_skb(NULL, len);
+ if (!skb)
+ return -ENOMEM;
- psize = (size + 3) & ~3;
+ cb = (struct qrtr_cb *)skb->cb;
- if (ver != QRTR_PROTO_VER)
- return -EINVAL;
+ /* Version field in v1 is little endian, so this works for both cases */
+ ver = *(u8*)data;
- if (len != psize + QRTR_HDR_SIZE)
- return -EINVAL;
+ switch (ver) {
+ case QRTR_PROTO_VER_1:
+ v1 = data;
+ hdrlen = sizeof(*v1);
- if (dst != QRTR_PORT_CTRL && type != QRTR_TYPE_DATA)
- return -EINVAL;
+ cb->type = le32_to_cpu(v1->type);
+ cb->src_node = le32_to_cpu(v1->src_node_id);
+ cb->src_port = le32_to_cpu(v1->src_port_id);
+ cb->confirm_rx = !!v1->confirm_rx;
+ cb->dst_node = le32_to_cpu(v1->dst_node_id);
+ cb->dst_port = le32_to_cpu(v1->dst_port_id);
- skb = netdev_alloc_skb(NULL, len);
- if (!skb)
- return -ENOMEM;
+ size = le32_to_cpu(v1->size);
+ break;
+ case QRTR_PROTO_VER_2:
+ v2 = data;
+ hdrlen = sizeof(*v2) + v2->optlen;
+
+ cb->type = v2->type;
+ cb->confirm_rx = !!(v2->flags & QRTR_FLAGS_CONFIRM_RX);
+ cb->src_node = le16_to_cpu(v2->src_node_id);
+ cb->src_port = le16_to_cpu(v2->src_port_id);
+ cb->dst_node = le16_to_cpu(v2->dst_node_id);
+ cb->dst_port = le16_to_cpu(v2->dst_port_id);
+
+ if (cb->src_port == (u16)QRTR_PORT_CTRL)
+ cb->src_port = QRTR_PORT_CTRL;
+ if (cb->dst_port == (u16)QRTR_PORT_CTRL)
+ cb->dst_port = QRTR_PORT_CTRL;
+
+ size = le32_to_cpu(v2->size);
+ break;
+ default:
+ pr_err("qrtr: Invalid version %d\n", ver);
+ goto err;
+ }
- cb = (struct qrtr_cb *)skb->cb;
- cb->src_node = le32_to_cpu(phdr->src_node_id);
- cb->src_port = le32_to_cpu(phdr->src_port_id);
- cb->dst_node = le32_to_cpu(phdr->dst_node_id);
- cb->dst_port = le32_to_cpu(phdr->dst_port_id);
- cb->type = type;
- cb->confirm_rx = !!phdr->confirm_rx;
+ if (len != ALIGN(size, 4) + hdrlen)
+ goto err;
- skb_put_data(skb, data + QRTR_HDR_SIZE, size);
+ if (cb->dst_port != QRTR_PORT_CTRL && cb->type != QRTR_TYPE_DATA)
+ goto err;
+
+ skb_put_data(skb, data + hdrlen, size);
skb_queue_tail(&node->rx_queue, skb);
schedule_work(&node->work);
return 0;
+
+err:
+ kfree_skb(skb);
+ return -EINVAL;
+
}
EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
@@ -287,11 +343,11 @@ static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt)
const int pkt_len = sizeof(struct qrtr_ctrl_pkt);
struct sk_buff *skb;
- skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL);
+ skb = alloc_skb(QRTR_HDR_MAX_SIZE + pkt_len, GFP_KERNEL);
if (!skb)
return NULL;
- skb_reserve(skb, QRTR_HDR_SIZE);
+ skb_reserve(skb, QRTR_HDR_MAX_SIZE);
*pkt = skb_put_zero(skb, pkt_len);
return skb;
@@ -720,12 +776,12 @@ static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
}
plen = (len + 3) & ~3;
- skb = sock_alloc_send_skb(sk, plen + QRTR_HDR_SIZE,
+ skb = sock_alloc_send_skb(sk, plen + QRTR_HDR_MAX_SIZE,
msg->msg_flags & MSG_DONTWAIT, &rc);
if (!skb)
goto out_node;
- skb_reserve(skb, QRTR_HDR_SIZE);
+ skb_reserve(skb, QRTR_HDR_MAX_SIZE);
rc = memcpy_from_msg(skb_put(skb, len), msg, len);
if (rc) {
--
2.12.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox