* [net-next 5/5] tipc: buffer overflow handling in listener socket
From: Jon Maloy @ 2018-09-28 18:23 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1538159002-11800-1-git-send-email-jon.maloy@ericsson.com>
From: Tung Nguyen <tung.q.nguyen@dektech.com.au>
Default socket receive buffer size for a listener socket is 2Mb. For
each arriving empty SYN, the linux kernel allocates a 768 bytes buffer.
This means that a listener socket can serve maximum 2700 simultaneous
empty connection setup requests before it hits a receive buffer
overflow, and much fewer if the SYN is carrying any significant
amount of data.
When this happens the setup request is rejected, and the client
receives an ECONNREFUSED error.
This commit mitigates this problem by letting the client socket try to
retransmit the SYN message multiple times when it sees it rejected with
the code TIPC_ERR_OVERLOAD. Retransmission is done at random intervals
in the range of [100 ms, setup_timeout / 4], as many times as there is
room for within the setup timeout limit.
Signed-off-by: Tung Nguyen <tung.q.nguyen@dektech.com.au>
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/msg.c | 20 ++++++++++++++++++++
net/tipc/msg.h | 1 +
net/tipc/socket.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
3 files changed, 64 insertions(+), 6 deletions(-)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 00fbb5c..f48e585 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -525,6 +525,10 @@ bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err)
if (hlen == SHORT_H_SIZE)
hlen = BASIC_H_SIZE;
+ /* Don't return data along with SYN+, - sender has a clone */
+ if (msg_is_syn(_hdr) && err == TIPC_ERR_OVERLOAD)
+ dlen = 0;
+
/* Allocate new buffer to return */
*skb = tipc_buf_acquire(hlen + dlen, GFP_ATOMIC);
if (!*skb)
@@ -552,6 +556,22 @@ bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err)
return false;
}
+bool tipc_msg_skb_clone(struct sk_buff_head *msg, struct sk_buff_head *cpy)
+{
+ struct sk_buff *skb, *_skb;
+
+ skb_queue_walk(msg, skb) {
+ _skb = skb_clone(skb, GFP_ATOMIC);
+ if (!_skb) {
+ __skb_queue_purge(cpy);
+ pr_err_ratelimited("Failed to clone buffer chain\n");
+ return false;
+ }
+ __skb_queue_tail(cpy, _skb);
+ }
+ return true;
+}
+
/**
* tipc_msg_lookup_dest(): try to find new destination for named message
* @skb: the buffer containing the message.
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index cd64d7b..a2879e6 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -980,6 +980,7 @@ bool tipc_msg_pskb_copy(u32 dst, struct sk_buff_head *msg,
struct sk_buff_head *cpy);
void __tipc_skb_queue_sorted(struct sk_buff_head *list, u16 seqno,
struct sk_buff *skb);
+bool tipc_msg_skb_clone(struct sk_buff_head *msg, struct sk_buff_head *cpy);
static inline u16 buf_seqno(struct sk_buff *skb)
{
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 89d6dc0..595c500 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -47,7 +47,7 @@
#include "netlink.h"
#include "group.h"
-#define CONN_TIMEOUT_DEFAULT 8000 /* default connect timeout = 8s */
+#define CONN_TIMEOUT_DEFAULT 8000 /* default connect timeout = 8s */
#define CONN_PROBING_INTV msecs_to_jiffies(3600000) /* [ms] => 1 h */
#define TIPC_FWD_MSG 1
#define TIPC_MAX_PORT 0xffffffff
@@ -80,7 +80,6 @@ struct sockaddr_pair {
* @publications: list of publications for port
* @blocking_link: address of the congested link we are currently sleeping on
* @pub_count: total # of publications port has made during its lifetime
- * @probing_state:
* @conn_timeout: the time we can wait for an unresponded setup request
* @dupl_rcvcnt: number of bytes counted twice, in both backlog and rcv queue
* @cong_link_cnt: number of congested links
@@ -102,8 +101,8 @@ struct tipc_sock {
struct list_head cong_links;
struct list_head publications;
u32 pub_count;
- uint conn_timeout;
atomic_t dupl_rcvcnt;
+ u16 conn_timeout;
bool probe_unacked;
u16 cong_link_cnt;
u16 snt_unacked;
@@ -507,6 +506,9 @@ static void __tipc_shutdown(struct socket *sock, int error)
tipc_wait_for_cond(sock, &timeout, (!tsk->cong_link_cnt &&
!tsk_conn_cong(tsk)));
+ /* Remove any pending SYN message */
+ __skb_queue_purge(&sk->sk_write_queue);
+
/* Reject all unreceived messages, except on an active connection
* (which disconnects locally & sends a 'FIN+' to peer).
*/
@@ -1362,6 +1364,8 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
rc = tipc_msg_build(hdr, m, 0, dlen, mtu, &pkts);
if (unlikely(rc != dlen))
return rc;
+ if (unlikely(syn && !tipc_msg_skb_clone(&pkts, &sk->sk_write_queue)))
+ return -ENOMEM;
rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
if (unlikely(rc == -ELINKCONG)) {
@@ -1491,6 +1495,7 @@ static void tipc_sk_finish_conn(struct tipc_sock *tsk, u32 peer_port,
tipc_node_add_conn(net, peer_node, tsk->portid, peer_port);
tsk->max_pkt = tipc_node_get_mtu(net, peer_node, tsk->portid);
tsk->peer_caps = tipc_node_get_capabilities(net, peer_node);
+ __skb_queue_purge(&sk->sk_write_queue);
if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
return;
@@ -1977,6 +1982,7 @@ static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
u32 oport = msg_origport(hdr);
u32 onode = msg_orignode(hdr);
int err = msg_errcode(hdr);
+ unsigned long delay;
if (unlikely(msg_mcast(hdr)))
return false;
@@ -2001,8 +2007,18 @@ static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
if (oport != pport || onode != pnode)
return false;
- /* Rejected SYN - abort */
- break;
+ /* Rejected SYN */
+ if (err != TIPC_ERR_OVERLOAD)
+ break;
+
+ /* Prepare for new setup attempt if we have a SYN clone */
+ if (skb_queue_empty(&sk->sk_write_queue))
+ break;
+ get_random_bytes(&delay, 2);
+ delay %= (tsk->conn_timeout / 4);
+ delay = msecs_to_jiffies(delay + 100);
+ sk_reset_timer(sk, &sk->sk_timer, jiffies + delay);
+ return false;
case TIPC_OPEN:
case TIPC_DISCONNECTING:
return false;
@@ -2561,12 +2577,26 @@ static void tipc_sk_check_probing_state(struct sock *sk,
sk_reset_timer(sk, &sk->sk_timer, jiffies + CONN_PROBING_INTV);
}
+static void tipc_sk_retry_connect(struct sock *sk, struct sk_buff_head *list)
+{
+ struct tipc_sock *tsk = tipc_sk(sk);
+
+ /* Try again later if dest link is congested */
+ if (tsk->cong_link_cnt) {
+ sk_reset_timer(sk, &sk->sk_timer, msecs_to_jiffies(100));
+ return;
+ }
+ /* Prepare SYN for retransmit */
+ tipc_msg_skb_clone(&sk->sk_write_queue, list);
+}
+
static void tipc_sk_timeout(struct timer_list *t)
{
struct sock *sk = from_timer(sk, t, sk_timer);
struct tipc_sock *tsk = tipc_sk(sk);
u32 pnode = tsk_peer_node(tsk);
struct sk_buff_head list;
+ int rc = 0;
skb_queue_head_init(&list);
bh_lock_sock(sk);
@@ -2580,12 +2610,19 @@ static void tipc_sk_timeout(struct timer_list *t)
if (sk->sk_state == TIPC_ESTABLISHED)
tipc_sk_check_probing_state(sk, &list);
+ else if (sk->sk_state == TIPC_CONNECTING)
+ tipc_sk_retry_connect(sk, &list);
bh_unlock_sock(sk);
if (!skb_queue_empty(&list))
- tipc_node_xmit(sock_net(sk), &list, pnode, tsk->portid);
+ rc = tipc_node_xmit(sock_net(sk), &list, pnode, tsk->portid);
+ /* SYN messages may cause link congestion */
+ if (rc == -ELINKCONG) {
+ tipc_dest_push(&tsk->cong_links, pnode, 0);
+ tsk->cong_link_cnt = 1;
+ }
sock_put(sk);
}
--
2.1.4
^ permalink raw reply related
* Re: bond: take rcu lock in bond_poll_controller
From: Dave Jones @ 2018-09-28 18:21 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpV67iFZptFfWPXqrFD6GW3JKR78GwocqnA1BeTS3fJaAA@mail.gmail.com>
On Fri, Sep 28, 2018 at 10:31:39AM -0700, Cong Wang wrote:
> On Fri, Sep 28, 2018 at 10:25 AM Dave Jones <davej@codemonkey.org.uk> wrote:
> >
> > On Fri, Sep 28, 2018 at 09:55:52AM -0700, Cong Wang wrote:
> > > On Fri, Sep 28, 2018 at 9:18 AM Dave Jones <davej@codemonkey.org.uk> wrote:
> > > >
> > > > Callers of bond_for_each_slave_rcu are expected to hold the rcu lock,
> > > > otherwise a trace like below is shown
> > >
> > > So why not take rcu read lock in netpoll_send_skb_on_dev() where
> > > RCU is also assumed?
> >
> > that does seem to solve the backtrace spew I saw too, so if that's
> > preferable I can respin the patch.
>
>
> >From my observations, netpoll_send_skb_on_dev() does not take
> RCU read lock _and_ it relies on rcu read lock because it calls
> rcu_dereference_bh().
>
> If my observation is correct, you should catch a RCU warning like
> this but within netpoll_send_skb_on_dev().
>
> > > As I said, I can't explain why you didn't trigger the RCU warning in
> > > netpoll_send_skb_on_dev()...
> >
> > netpoll_send_skb_on_dev takes the rcu lock itself.
>
> Could you please point me where exactly is the rcu lock here?
>
> I am too stupid to see it. :)
No, I'm the stupid one. I looked at the tree I had just edited to try your
proposed change.
Now that I've untangled myself, I'll repost with your suggested change.
Dave
^ permalink raw reply
* [PATCH net v2] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-09-28 18:15 UTC (permalink / raw)
To: joel, sam; +Cc: linux-aspeed, netdev, openbmc, amithash, christian, vijaykhemka
The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
to send NC-SI command to the network card.
Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
The work flow is as below.
Request:
User space application -> Netlink interface (msg)
-> new Netlink handler - ncsi_send_cmd_nl()
-> ncsi_xmit_cmd()
Response:
Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx()
-> ncsi_rsp_handler_netlink()
-> ncsi_send_netlink_rsp ()
-> Netlink interface (msg)
-> user space application
Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
-> Netlink interface (msg with zero data length)
-> user space application
Error:
Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
-> user space application
Signed-off-by: Justin Lee <justin.lee1@dell.com>
---
include/uapi/linux/ncsi.h | 3 +
net/ncsi/internal.h | 12 ++-
net/ncsi/ncsi-cmd.c | 47 ++++++++++-
net/ncsi/ncsi-manage.c | 22 +++++
net/ncsi/ncsi-netlink.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-netlink.h | 12 +++
net/ncsi/ncsi-rsp.c | 71 ++++++++++++++--
7 files changed, 363 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..4992bfc 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -30,6 +30,7 @@ enum ncsi_nl_commands {
NCSI_CMD_PKG_INFO,
NCSI_CMD_SET_INTERFACE,
NCSI_CMD_CLEAR_INTERFACE,
+ NCSI_CMD_SEND_CMD,
__NCSI_CMD_AFTER_LAST,
NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
@@ -43,6 +44,7 @@ enum ncsi_nl_commands {
* @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
* @NCSI_ATTR_PACKAGE_ID: package ID
* @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_DATA: command payload
* @NCSI_ATTR_MAX: highest attribute number
*/
enum ncsi_nl_attrs {
@@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
NCSI_ATTR_PACKAGE_LIST,
NCSI_ATTR_PACKAGE_ID,
NCSI_ATTR_CHANNEL_ID,
+ NCSI_ATTR_DATA,
__NCSI_ATTR_AFTER_LAST,
NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8055e39..1a3ef9e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -171,6 +171,8 @@ struct ncsi_package;
#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
+#define NCSI_MAX_PACKAGE 8
+#define NCSI_MAX_CHANNEL 32
struct ncsi_channel {
unsigned char id;
@@ -215,12 +217,17 @@ struct ncsi_request {
unsigned char id; /* Request ID - 0 to 255 */
bool used; /* Request that has been assigned */
unsigned int flags; /* NCSI request property */
-#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
struct timer_list timer; /* Timer on waiting for response */
bool enabled; /* Time has been enabled or not */
+
+ u32 snd_seq; /* netlink sending sequence number */
+ u32 snd_portid; /* netlink portid of sender */
+ struct nlmsghdr nlhdr; /* netlink message header */
};
enum {
@@ -305,6 +312,9 @@ struct ncsi_cmd_arg {
unsigned short words[8];
unsigned int dwords[4];
};
+
+ unsigned char *data; /* Netlink data */
+ struct genl_info *info; /* Netlink information */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 7567ca63..43b544c 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -17,6 +17,7 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -211,6 +212,39 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
return 0;
}
+static int ncsi_cmd_handler_oem(struct sk_buff *skb,
+ struct ncsi_cmd_arg *nca)
+{
+ struct ncsi_cmd_pkt *cmd;
+ unsigned char *dest, *source;
+ unsigned short len;
+
+ /* struct ncsi_cmd_pkt = minimum length
+ * - frame checksum
+ * - Ethernet header
+ * = 64 - 4 - 14 = 46
+ * minimum payload = 46 - ncsi header - ncsi checksum
+ * = 46 - 16 - 4 = 26
+ */
+ len = nca->payload;
+
+ /* minimum payload length is 26 bytes to meet minimum packet
+ * length 64
+ */
+ if (len < 26)
+ cmd = skb_put_zero(skb, sizeof(*cmd));
+ else
+ cmd = skb_put_zero(skb, len + sizeof(struct ncsi_pkt_hdr) + 4);
+
+ dest = (unsigned char *)cmd + sizeof(struct ncsi_pkt_hdr);
+ source = (unsigned char *)nca->data + sizeof(struct ncsi_pkt_hdr);
+ memcpy(dest, source, len);
+
+ ncsi_cmd_build_header(&cmd->cmd.common, nca);
+
+ return 0;
+}
+
static struct ncsi_cmd_handler {
unsigned char type;
int payload;
@@ -244,7 +278,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_OEM, 0, NULL },
+ { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
{ NCSI_PKT_CMD_PLDM, 0, NULL },
{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
};
@@ -317,11 +351,20 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
}
/* Get packet payload length and allocate the request */
- nca->payload = nch->payload;
+ if (nch->payload >= 0)
+ nca->payload = nch->payload;
+
nr = ncsi_alloc_command(nca);
if (!nr)
return -ENOMEM;
+ /* track netlink information */
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ nr->snd_seq = nca->info->snd_seq;
+ nr->snd_portid = nca->info->snd_portid;
+ nr->nlhdr = *nca->info->nlhdr;
+ }
+
/* Prepare the packet */
nca->id = nr->id;
ret = nch->handler(nr->cmd, nca);
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..29f33a1 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
#include <net/addrconf.h>
#include <net/ipv6.h>
#include <net/if_inet6.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
@@ -406,8 +407,13 @@ static void ncsi_request_timeout(struct timer_list *t)
{
struct ncsi_request *nr = from_timer(nr, t, timer);
struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ struct ncsi_cmd_pkt *cmd;
unsigned long flags;
+ netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
+
/* If the request already had associated response,
* let the response handler to release it.
*/
@@ -415,10 +421,26 @@ static void ncsi_request_timeout(struct timer_list *t)
nr->enabled = false;
if (nr->rsp || !nr->cmd) {
spin_unlock_irqrestore(&ndp->lock, flags);
+
+ netdev_dbg(ndp->ndev.dev,
+ "NCSI: %s - early return\n", __func__);
+
return;
}
spin_unlock_irqrestore(&ndp->lock, flags);
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (nr->cmd) {
+ /* Find the package */
+ cmd = (struct ncsi_cmd_pkt *)
+ skb_network_header(nr->cmd);
+ ncsi_find_package_and_channel(ndp,
+ cmd->cmd.common.channel,
+ &np, &nc);
+ ncsi_send_netlink_timeout(nr, np, nc);
+ }
+ }
+
/* Release the request */
ncsi_free_request(nr);
}
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..ce57675 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
#include <uapi/linux/ncsi.h>
#include "internal.h"
+#include "ncsi-pkt.h"
#include "ncsi-netlink.h"
static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
[NCSI_ATTR_PACKAGE_LIST] = { .type = NLA_NESTED },
[NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 },
[NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 },
+ [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 },
};
static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,203 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
return 0;
}
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+ struct ncsi_dev_priv *ndp;
+
+ struct ncsi_cmd_arg nca;
+ struct ncsi_pkt_hdr *hdr;
+
+ u32 package_id, channel_id;
+ unsigned char *data;
+ void *head;
+ int len, ret;
+
+ if (!info || !info->attrs) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+ nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+ if (!ndp) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+ channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+ if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+ ret = -ERANGE;
+ goto out_netlink;
+ }
+
+ len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+ if (len < sizeof(struct ncsi_pkt_hdr)) {
+ netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
+ package_id);
+ ret = -EINVAL;
+ goto out_netlink;
+ } else {
+ head = nla_data(info->attrs[NCSI_ATTR_DATA]);
+ data = (unsigned char *)head;
+ }
+
+ hdr = (struct ncsi_pkt_hdr *)data;
+
+ nca.ndp = ndp;
+ nca.package = (unsigned char)package_id;
+ nca.channel = (unsigned char)channel_id;
+ nca.type = hdr->type;
+ nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+ nca.info = info;
+ nca.payload = ntohs(hdr->length);
+ nca.data = data;
+
+ ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+ if (ret != 0) {
+ netdev_err(ndp->ndev.dev,
+ "Error %d sending OEM command\n", ret);
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ info->snd_seq,
+ info->snd_portid,
+ info->nlhdr,
+ ret);
+ }
+out:
+ return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+ int rc;
+
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+ net = dev_net(nr->rsp->dev);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+ if (rc)
+ goto err;
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+ kfree_skb(skb);
+ return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc)
+{
+ struct sk_buff *skb;
+ struct net *net;
+ void *hdr;
+
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+ skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+ &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+ if (!hdr) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
+ net = dev_net(nr->cmd->dev);
+
+ nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+ if (np)
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+ NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+ nr->cmd->data)->channel)));
+
+ if (nc)
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+ else
+ nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ struct nlmsgerr *nle;
+ struct net *net;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ net = dev_net(dev);
+
+ nlh = nlmsg_put(skb, snd_portid, snd_seq,
+ NLMSG_ERROR, sizeof(*nle), 0);
+ nle = (struct nlmsgerr *)nlmsg_data(nlh);
+ nle->error = err;
+ memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+ nlmsg_end(skb, nlh);
+
+ return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
static const struct genl_ops ncsi_ops[] = {
{
.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +585,12 @@ static const struct genl_ops ncsi_ops[] = {
.doit = ncsi_clear_interface_nl,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NCSI_CMD_SEND_CMD,
+ .policy = ncsi_genl_policy,
+ .doit = ncsi_send_cmd_nl,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
#include "internal.h"
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+ struct ncsi_package *np,
+ struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+ u32 snd_seq,
+ u32 snd_portid,
+ struct nlmsghdr *nlhdr,
+ int err);
+
int ncsi_init_netlink(struct net_device *dev);
int ncsi_unregister_netlink(struct net_device *dev);
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 930c1d3..010970f 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
#include <net/ncsi.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <net/genetlink.h>
#include "internal.h"
#include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
@@ -32,15 +34,22 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
* before calling this function.
*/
h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
- if (h->common.revision != NCSI_PKT_REVISION)
+
+ if (h->common.revision != NCSI_PKT_REVISION) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: unsupported header revision\n");
return -EINVAL;
- if (ntohs(h->common.length) != payload)
+ }
+ if (ntohs(h->common.length) != payload) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: payload length mismatched\n");
return -EINVAL;
+ }
/* Check on code and reason */
if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
- ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
- return -EINVAL;
+ ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: non zero response/reason code\n");
+ return -EPERM;
+ }
/* Validate checksum, which might be zeroes if the
* sender doesn't support checksum according to NCSI
@@ -52,8 +61,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
checksum = ncsi_calculate_checksum((unsigned char *)h,
sizeof(*h) + payload - 4);
- if (*pchecksum != htonl(checksum))
+
+ if (*pchecksum != htonl(checksum)) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
return -EINVAL;
+ }
return 0;
}
@@ -900,6 +912,31 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
return 0;
}
+static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
+{
+ return 0;
+}
+
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ int ret;
+
+ /* Find the package */
+ rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+ ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+ &np, &nc);
+ if (!np)
+ return -ENODEV;
+
+ ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+ return ret;
+}
+
static struct ncsi_rsp_handler {
unsigned char type;
int payload;
@@ -932,7 +969,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
{ NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
- { NCSI_PKT_RSP_OEM, 0, NULL },
+ { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
{ NCSI_PKT_RSP_PLDM, 0, NULL },
{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
};
@@ -1002,6 +1039,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for type 0x%x\n",
hdr->type);
+
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ if (ret == -EPERM)
+ goto out_netlink;
+ else
+ ncsi_send_netlink_err(ndp->ndev.dev,
+ nr->snd_seq,
+ nr->snd_portid,
+ &nr->nlhdr,
+ ret);
+ }
goto out;
}
@@ -1011,6 +1059,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
netdev_err(ndp->ndev.dev,
"NCSI: Handler for packet type 0x%x returned %d\n",
hdr->type, ret);
+
+out_netlink:
+ if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ ret = ncsi_rsp_handler_netlink(nr);
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+ hdr->type, ret);
+ }
+ }
+
out:
ncsi_free_request(nr);
return ret;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net 00/11] netpoll: second round of fixes.
From: David Miller @ 2018-09-28 18:14 UTC (permalink / raw)
To: edumazet
Cc: netdev, michael.chan, aviad.krawczyk, songliubraving, dougmill,
yisen.zhuang, mst, jasowang, harish.patil, manish.chopra, netanel,
linux-net-drivers, tlfalcon
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 27 Sep 2018 09:31:50 -0700
> As diagnosed by Song Liu, ndo_poll_controller() can
> be very dangerous on loaded hosts, since the cpu
> calling ndo_poll_controller() might steal all NAPI
> contexts (for all RX/TX queues of the NIC).
>
> This capture, showing one ksoftirqd eating all cycles
> can last for unlimited amount of time, since one
> cpu is generally not able to drain all the queues under load.
>
> It seems that all networking drivers that do use NAPI
> for their TX completions, should not provide a ndo_poll_controller() :
>
> Most NAPI drivers have netpoll support already handled
> in core networking stack, since netpoll_poll_dev()
> uses poll_napi(dev) to iterate through registered
> NAPI contexts for a device.
>
> First patch is a fix in poll_one_napi().
>
> Then following patches take care of ten drivers.
Series applied, thanks Eric.
^ permalink raw reply
* Re: [net-next 0/8][pull request] 100GbE Intel Wired LAN Driver Updates 2018-09-27
From: David Miller @ 2018-09-28 18:11 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180927162201.30900-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 27 Sep 2018 09:21:53 -0700
> This series contains fixes to the ice driver only.
...
> The following are changes since commit 1042caa79e9351b81ed19dc8d2d7fd6ff51a4422:
> net-ipv4: remove 2 always zero parameters from ipv4_redirect()
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock
From: Ido Schimmel @ 2018-09-28 18:11 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <CAM_iQpUav4UK6uzpaZejFiLn_eEbbvZQry7G6JgzSkj_Eq0fyQ@mail.gmail.com>
On Fri, Sep 28, 2018 at 10:56:47AM -0700, Cong Wang wrote:
> On Fri, Sep 28, 2018 at 7:59 AM Ido Schimmel <idosch@idosch.org> wrote:
> >
> > On Wed, Sep 19, 2018 at 04:37:29PM -0700, Cong Wang wrote:
> > > From: Vlad Buslov <vladbu@mellanox.com>
> > >
> > > From: Vlad Buslov <vladbu@mellanox.com>
> > >
> > > Action API was changed to work with actions and action_idr in concurrency
> > > safe manner, however tcf_del_walker() still uses actions without taking a
> > > reference or idrinfo->lock first, and deletes them directly, disregarding
> > > possible concurrent delete.
> > >
> > > Change tcf_del_walker() to take idrinfo->lock while iterating over actions
> > > and use new tcf_idr_release_unsafe() to release them while holding the
> > > lock.
> > >
> > > And the blocking function fl_hw_destroy_tmplt() could be called when we
> > > put a filter chain, so defer it to a work queue.
> >
> > I'm getting a use-after-free when running tc_chains.sh selftest and I
> > believe it's caused by this patch.
> >
> > To reproduce:
> > # cd tools/testing/selftests/net/forwarding
> > # export TESTS="template_filter_fits"; ./tc_chains.sh veth0 veth1
> >
> > __tcf_chain_put()
> > tc_chain_tmplt_del()
> > fl_tmplt_destroy()
> > tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work)
> > tcf_chain_destroy()
> > kfree(chain)
> >
> > Some time later fl_tmplt_destroy_work() starts executing and
> > dereferencing 'chain'.
>
> Oops, forgot to hold the chain... I will test this:
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 92dd5071a708..cbb68d5515d6 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1444,6 +1444,7 @@ static void fl_tmplt_destroy_work(struct
> work_struct *work)
> struct fl_flow_tmplt, rwork);
>
> fl_hw_destroy_tmplt(tmplt->chain, tmplt);
> + tcf_chain_put(tmplt->chain);
> kfree(tmplt);
> }
>
> @@ -1451,6 +1452,7 @@ static void fl_tmplt_destroy(void *tmplt_priv)
> {
> struct fl_flow_tmplt *tmplt = tmplt_priv;
>
> + tcf_chain_hold(tmplt->chain);
> tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work);
> }
I don't think this will work given the reference count already dropped
to 0, which is why the template deletion function was invoked. I didn't
test the patch, but I don't see what would prevent the chain from being
freed.
Thanks for looking into this.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: make function qdisc_free_cb() static
From: David Miller @ 2018-09-28 18:06 UTC (permalink / raw)
To: weiyongjun1; +Cc: jhs, xiyou.wangcong, jiri, vladbu, netdev, kernel-janitors
In-Reply-To: <1538059676-71316-1-git-send-email-weiyongjun1@huawei.com>
From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Thu, 27 Sep 2018 14:47:56 +0000
> Fixes the following sparse warning:
>
> net/sched/sch_generic.c:944:6: warning:
> symbol 'qdisc_free_cb' was not declared. Should it be static?
>
> Fixes: 3a7d0d07a386 ("net: sched: extend Qdisc with rcu")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] geneve: fix ttl inherit type
From: David Ahern @ 2018-09-28 17:59 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: David Miller, Stephen Hemminger, Phil Sutter
In-Reply-To: <1538096998-20937-1-git-send-email-liuhangbin@gmail.com>
On 9/27/18 7:09 PM, Hangbin Liu wrote:
> Phil pointed out that there is a mismatch between vxlan and geneve ttl
> inherit. We should define it as a flag and use nla_put_flag to export this
> opiton.
>
> Fixes: 52d0d404d39dd ("geneve: add ttl inherit support")
same here .. getting an unknown commit id.
> Reported-by: Phil Sutter <phil@nwl.cc>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/geneve.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 6625fab..09ab2fd 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1100,7 +1100,7 @@ static const struct nla_policy geneve_policy[IFLA_GENEVE_MAX + 1] = {
> [IFLA_GENEVE_UDP_CSUM] = { .type = NLA_U8 },
> [IFLA_GENEVE_UDP_ZERO_CSUM6_TX] = { .type = NLA_U8 },
> [IFLA_GENEVE_UDP_ZERO_CSUM6_RX] = { .type = NLA_U8 },
> - [IFLA_GENEVE_TTL_INHERIT] = { .type = NLA_U8 },
> + [IFLA_GENEVE_TTL_INHERIT] = { .type = NLA_FLAG },
> };
>
> static int geneve_validate(struct nlattr *tb[], struct nlattr *data[],
> @@ -1582,7 +1582,7 @@ static size_t geneve_get_size(const struct net_device *dev)
> nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_UDP_CSUM */
> nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_UDP_ZERO_CSUM6_TX */
> nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_UDP_ZERO_CSUM6_RX */
> - nla_total_size(sizeof(__u8)) + /* IFLA_GENEVE_TTL_INHERIT */
> + nla_total_size(0) + /* IFLA_GENEVE_TTL_INHERIT */
> 0;
> }
>
> @@ -1636,7 +1636,7 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
> goto nla_put_failure;
> #endif
>
> - if (nla_put_u8(skb, IFLA_GENEVE_TTL_INHERIT, ttl_inherit))
> + if (ttl_inherit && nla_put_flag(skb, IFLA_GENEVE_TTL_INHERIT))
> goto nla_put_failure;
>
> return 0;
>
^ permalink raw reply
* Re: [PATCH net] vxlan: use nla_put_flag for ttl inherit
From: David Ahern @ 2018-09-28 17:57 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: David Miller, Stephen Hemminger, Phil Sutter
In-Reply-To: <1538096906-20866-1-git-send-email-liuhangbin@gmail.com>
On 9/27/18 7:08 PM, Hangbin Liu wrote:
> Phil pointed out that there is a mismatch between vxlan and geneve ttl inherit.
> We should define it as a flag and use nla_put_flag to export this opiton.
>
> Fixes: 8fd780698745b ("vxlan: fill ttl inherit info")
Wrong Fixes tag:
kenny:mgmt:iproute2-next.git$ git show 8fd780698745b
fatal: ambiguous argument '8fd780698745b': unknown revision or path not
in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
> Reported-by: Phil Sutter <phil@nwl.cc>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/vxlan.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 2b8da2b..479dda4 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3539,7 +3539,7 @@ static size_t vxlan_get_size(const struct net_device *dev)
> nla_total_size(sizeof(__u32)) + /* IFLA_VXLAN_LINK */
> nla_total_size(sizeof(struct in6_addr)) + /* IFLA_VXLAN_LOCAL{6} */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TTL */
> - nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TTL_INHERIT */
> + nla_total_size(0) + /* IFLA_VXLAN_TTL_INHERIT */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TOS */
> nla_total_size(sizeof(__be32)) + /* IFLA_VXLAN_LABEL */
> nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_LEARNING */
> @@ -3604,8 +3604,6 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
> }
>
> if (nla_put_u8(skb, IFLA_VXLAN_TTL, vxlan->cfg.ttl) ||
> - nla_put_u8(skb, IFLA_VXLAN_TTL_INHERIT,
> - !!(vxlan->cfg.flags & VXLAN_F_TTL_INHERIT)) ||
> nla_put_u8(skb, IFLA_VXLAN_TOS, vxlan->cfg.tos) ||
> nla_put_be32(skb, IFLA_VXLAN_LABEL, vxlan->cfg.label) ||
> nla_put_u8(skb, IFLA_VXLAN_LEARNING,
> @@ -3650,6 +3648,10 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
> nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
> goto nla_put_failure;
>
> + if (vxlan->cfg.flags & VXLAN_F_TTL_INHERIT &&
> + nla_put_flag(skb, IFLA_VXLAN_TTL_INHERIT))
> + goto nla_put_failure;
> +
> return 0;
>
> nla_put_failure:
>
^ permalink raw reply
* Re: [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock
From: Cong Wang @ 2018-09-28 17:56 UTC (permalink / raw)
To: Ido Schimmel
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <20180928145900.GA17640@splinter>
On Fri, Sep 28, 2018 at 7:59 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Wed, Sep 19, 2018 at 04:37:29PM -0700, Cong Wang wrote:
> > From: Vlad Buslov <vladbu@mellanox.com>
> >
> > From: Vlad Buslov <vladbu@mellanox.com>
> >
> > Action API was changed to work with actions and action_idr in concurrency
> > safe manner, however tcf_del_walker() still uses actions without taking a
> > reference or idrinfo->lock first, and deletes them directly, disregarding
> > possible concurrent delete.
> >
> > Change tcf_del_walker() to take idrinfo->lock while iterating over actions
> > and use new tcf_idr_release_unsafe() to release them while holding the
> > lock.
> >
> > And the blocking function fl_hw_destroy_tmplt() could be called when we
> > put a filter chain, so defer it to a work queue.
>
> I'm getting a use-after-free when running tc_chains.sh selftest and I
> believe it's caused by this patch.
>
> To reproduce:
> # cd tools/testing/selftests/net/forwarding
> # export TESTS="template_filter_fits"; ./tc_chains.sh veth0 veth1
>
> __tcf_chain_put()
> tc_chain_tmplt_del()
> fl_tmplt_destroy()
> tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work)
> tcf_chain_destroy()
> kfree(chain)
>
> Some time later fl_tmplt_destroy_work() starts executing and
> dereferencing 'chain'.
Oops, forgot to hold the chain... I will test this:
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 92dd5071a708..cbb68d5515d6 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1444,6 +1444,7 @@ static void fl_tmplt_destroy_work(struct
work_struct *work)
struct fl_flow_tmplt, rwork);
fl_hw_destroy_tmplt(tmplt->chain, tmplt);
+ tcf_chain_put(tmplt->chain);
kfree(tmplt);
}
@@ -1451,6 +1452,7 @@ static void fl_tmplt_destroy(void *tmplt_priv)
{
struct fl_flow_tmplt *tmplt = tmplt_priv;
+ tcf_chain_hold(tmplt->chain);
tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work);
}
^ permalink raw reply related
* Re: [PATCH net] vxlan: use nla_put_flag for ttl inherit
From: David Ahern @ 2018-09-28 17:56 UTC (permalink / raw)
To: Hangbin Liu, David Miller; +Cc: Phil Sutter, netdev, Stephen Hemminger
In-Reply-To: <20180928123858.GF24677@leo.usersys.redhat.com>
On 9/28/18 6:38 AM, Hangbin Liu wrote:
> On Fri, Sep 28, 2018 at 12:37:00PM +0200, Phil Sutter wrote:
>> On Fri, Sep 28, 2018 at 09:08:26AM +0800, Hangbin Liu wrote:
>>> Phil pointed out that there is a mismatch between vxlan and geneve ttl inherit.
>>> We should define it as a flag and use nla_put_flag to export this opiton.
>>
>> s/opiton/option/
>>
>> Apart from that, LGTM!
>>
>> Thanks, Phil
>
> Opps, sorry...
>
> Hi David,
>
> Should I re-send a patch or will you help fix it directly?
>
> Thanks
> Hangbin
>
you have this targeted at net; is it a bug in current iproute2 or an
update to a new feature in -next?
^ permalink raw reply
* [PATCH] Revert "openvswitch: Fix template leak in error cases."
From: Flavio Leitner @ 2018-09-28 17:55 UTC (permalink / raw)
To: netfilter-devel, netdev, dev
Cc: Pravin B Shelar, Joe Stringer, Flavio Leitner
This reverts commit 90c7afc96cbbd77f44094b5b651261968e97de67.
When the commit was merged, the code used nf_ct_put() to free
the entry, but later on commit 76644232e612 ("openvswitch: Free
tmpl with tmpl_free.") replaced that with nf_ct_tmpl_free which
is a more appropriate. Now the original problem is removed.
Then 44d6e2f27328 ("net: Replace NF_CT_ASSERT() with WARN_ON().")
replaced a debug assert with a WARN_ON() which is trigged now.
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
net/openvswitch/conntrack.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 86a75105af1a..0aeb34c6389d 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1624,10 +1624,6 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr,
OVS_NLERR(log, "Failed to allocate conntrack template");
return -ENOMEM;
}
-
- __set_bit(IPS_CONFIRMED_BIT, &ct_info.ct->status);
- nf_conntrack_get(&ct_info.ct->ct_general);
-
if (helper) {
err = ovs_ct_add_helper(&ct_info, helper, key, log);
if (err)
@@ -1639,6 +1635,8 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr,
if (err)
goto err_free_ct;
+ __set_bit(IPS_CONFIRMED_BIT, &ct_info.ct->status);
+ nf_conntrack_get(&ct_info.ct->ct_general);
return 0;
err_free_ct:
__ovs_ct_free_action(&ct_info);
--
2.14.4
^ permalink raw reply related
* Re: [PATCH iproute2 net-next] bridge: fdb: add support for sticky flag
From: David Ahern @ 2018-09-28 17:54 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: roopa
In-Reply-To: <20180927133513.14175-1-nikolay@cumulusnetworks.com>
On 9/27/18 7:35 AM, Nikolay Aleksandrov wrote:
> Add support for the new sticky flag that can be set on fdbs and update the
> man page.
>
> CC: David Ahern <dsahern@gmail.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> bridge/fdb.c | 9 +++++++--
> man/man8/bridge.8 | 6 +++++-
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply
* [PATCH] openvswitch: load NAT helper
From: Flavio Leitner @ 2018-09-28 17:51 UTC (permalink / raw)
To: netfilter-devel, netdev, dev; +Cc: Pravin B Shelar, Flavio Leitner
Load the respective NAT helper module if the flow uses it.
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
net/openvswitch/conntrack.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 86a75105af1a..7e9a5283e236 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1312,6 +1312,10 @@ static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
rcu_assign_pointer(help->helper, helper);
info->helper = helper;
+
+ if (info->nat)
+ request_module("ip_nat_%s", name);
+
return 0;
}
--
2.14.4
^ permalink raw reply related
* Re: [PATCH 04/11] net: ip6_multipath_l3_keys() - use new style struct initializer instead of memset
From: David Ahern @ 2018-09-28 17:48 UTC (permalink / raw)
To: Maciej Żenczykowski, Maciej Żenczykowski,
David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-4-zenczykowski@gmail.com>
On 9/27/18 5:00 PM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
> net/ipv6/route.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index d28f83e01593..9cb024451fc5 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1981,12 +1981,11 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb,
> u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
> const struct sk_buff *skb, struct flow_keys *flkeys)
> {
> - struct flow_keys hash_keys;
> + struct flow_keys hash_keys = {};
> u32 mhash;
>
> switch (ip6_multipath_hash_policy(net)) {
> case 0:
> - memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> if (skb) {
> ip6_multipath_l3_keys(skb, &hash_keys, flkeys);
> @@ -2006,8 +2005,6 @@ u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
> if (skb->l4_hash)
> return skb_get_hash_raw(skb) >> 1;
>
> - memset(&hash_keys, 0, sizeof(hash_keys));
> -
> if (!flkeys) {
> skb_flow_dissect_flow_keys(skb, &keys, flag);
> flkeys = &keys;
> @@ -2019,7 +2016,6 @@ u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
> hash_keys.ports.dst = flkeys->ports.dst;
> hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
> } else {
> - memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
> hash_keys.addrs.v6addrs.src = fl6->saddr;
> hash_keys.addrs.v6addrs.dst = fl6->daddr;
>
ditto for this one.
^ permalink raw reply
* Re: [PATCH 03/11] net: fib_multipath_hash() - use new style struct initializer instead of memset
From: David Ahern @ 2018-09-28 17:47 UTC (permalink / raw)
To: Maciej Żenczykowski, Maciej Żenczykowski,
David S . Miller; +Cc: netdev
In-Reply-To: <20180927230017.15398-3-zenczykowski@gmail.com>
On 9/27/18 5:00 PM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
> net/ipv4/route.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 048919713f4e..17953a52fbd0 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1821,12 +1821,11 @@ static void ip_multipath_l3_keys(const struct sk_buff *skb,
> int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
> const struct sk_buff *skb, struct flow_keys *flkeys)
> {
> - struct flow_keys hash_keys;
> + struct flow_keys hash_keys = {};
> u32 mhash;
>
> switch (net->ipv4.sysctl_fib_multipath_hash_policy) {
> case 0:
> - memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> if (skb) {
> ip_multipath_l3_keys(skb, &hash_keys);
> @@ -1845,8 +1844,6 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
> if (skb->l4_hash)
> return skb_get_hash_raw(skb) >> 1;
>
> - memset(&hash_keys, 0, sizeof(hash_keys));
> -
> if (!flkeys) {
> skb_flow_dissect_flow_keys(skb, &keys, flag);
> flkeys = &keys;
> @@ -1859,7 +1856,6 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
> hash_keys.ports.dst = flkeys->ports.dst;
> hash_keys.basic.ip_proto = flkeys->basic.ip_proto;
> } else {
> - memset(&hash_keys, 0, sizeof(hash_keys));
> hash_keys.control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> hash_keys.addrs.v4addrs.src = fl4->saddr;
> hash_keys.addrs.v4addrs.dst = fl4->daddr;
>
NACK on this one.
This is the hot path and the memset was done right before use for least
overhead.
^ permalink raw reply
* Re: [PATCH net-next] selftests: forwarding: test for bridge sticky flag
From: David Miller @ 2018-09-28 17:45 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa
In-Reply-To: <20180927133513.14175-2-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 27 Sep 2018 16:35:13 +0300
> This test adds an fdb entry with the sticky flag and sends traffic from
> a different port with the same mac as a source address expecting the entry
> to not change ports if the flag is operating correctly.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: bridge: explicitly zero is_sticky in fdb_create
From: David Miller @ 2018-09-28 17:43 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, bridge
In-Reply-To: <20180927120510.7314-1-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 27 Sep 2018 15:05:10 +0300
> We need to explicitly zero is_sticky when creating a new fdb, otherwise
> we might get a stale value for a new entry.
>
> Fixes: 435f2e7cc0b7 ("net: bridge: add support for sticky fdb entries")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied, thanks Nikolay.
^ permalink raw reply
* Re: pull-request: mac80211 2018-09-27
From: David Miller @ 2018-09-28 17:42 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20180927111851.8058-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 27 Sep 2018 13:18:50 +0200
> Here's another - unfortunately pretty large - set of fixes
> for the current cycle. The changes are pretty simple or even
> trivial though.
>
> Please pull and let me know if there's any problem.
Pulled.
If they are all legit fixes, how can I object, right? :)
Thanks!
^ permalink raw reply
* Re: [PATCH net 1/1] qed: Fix shmem structure inconsistency between driver and the mfw.
From: David Miller @ 2018-09-28 17:40 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Michal.Kalderon
In-Reply-To: <20180927111210.14203-1-sudarsana.kalluru@cavium.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com>
Date: Thu, 27 Sep 2018 04:12:10 -0700
> The structure shared between driver and the management FW (mfw) differ in
> sizes. This would lead to issues when driver try to access the structure
> members which are not-aligned with the mfw copy e.g., data_ptr usage in the
> case of mfw_tlv request.
> Align the driver structure with mfw copy, add reserved field(s) to driver
> structure for the members not used by the driver.
>
> Fixes: dd006921d ("qed: Add MFW interfaces for TLV request support.)
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Applied and queued up for -stable.
In the future please use 12 digits of SHA1_ID for Fixes tags. I fixed
it up for you this time.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] Update maintainers for bnx2/bnx2x/qlge/qlcnic drivers.
From: David Miller @ 2018-09-28 17:32 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Ameen.Rahman
In-Reply-To: <20180927045703.6700-1-sudarsana.kalluru@cavium.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@cavium.com>
Date: Wed, 26 Sep 2018 21:57:03 -0700
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> Signed-off-by: Ameen Rahman <Ameen.Rahman@cavium.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] [PATCH net-next] openvswitch: Use correct reply values in datapath and vport ops
From: Pravin Shelar @ 2018-09-28 17:12 UTC (permalink / raw)
To: Yifeng Sun; +Cc: Linux Kernel Network Developers
In-Reply-To: <1537987214-6414-1-git-send-email-pkusunyifeng@gmail.com>
On Wed, Sep 26, 2018 at 11:40 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote:
>
> This patch fixes the bug that all datapath and vport ops are returning
> wrong values (OVS_FLOW_CMD_NEW or OVS_DP_CMD_NEW) in their replies.
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
I am surprised this was not found earlier.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Thanks.
^ permalink raw reply
* Re: bond: take rcu lock in bond_poll_controller
From: Cong Wang @ 2018-09-28 17:31 UTC (permalink / raw)
To: Dave Jones; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180928172556.arqk7ntmnz6lhb6d@codemonkey.org.uk>
On Fri, Sep 28, 2018 at 10:25 AM Dave Jones <davej@codemonkey.org.uk> wrote:
>
> On Fri, Sep 28, 2018 at 09:55:52AM -0700, Cong Wang wrote:
> > On Fri, Sep 28, 2018 at 9:18 AM Dave Jones <davej@codemonkey.org.uk> wrote:
> > >
> > > Callers of bond_for_each_slave_rcu are expected to hold the rcu lock,
> > > otherwise a trace like below is shown
> >
> > So why not take rcu read lock in netpoll_send_skb_on_dev() where
> > RCU is also assumed?
>
> that does seem to solve the backtrace spew I saw too, so if that's
> preferable I can respin the patch.
>From my observations, netpoll_send_skb_on_dev() does not take
RCU read lock _and_ it relies on rcu read lock because it calls
rcu_dereference_bh().
If my observation is correct, you should catch a RCU warning like
this but within netpoll_send_skb_on_dev().
>
> > As I said, I can't explain why you didn't trigger the RCU warning in
> > netpoll_send_skb_on_dev()...
>
> netpoll_send_skb_on_dev takes the rcu lock itself.
Could you please point me where exactly is the rcu lock here?
I am too stupid to see it. :)
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Edward Cree @ 2018-09-28 17:30 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ariel Almog, linville, Linux Netdev List, ganeshgr,
jakub.kicinski, dustin, dirk.vandermerwe, shayag, ariela
In-Reply-To: <20180928164553.GB22858@lunn.ch>
On 28/09/18 17:45, Andrew Lunn wrote:
> Now is a good time to change the API, since we are moving to a netlink
> socket. Which is why these questions were asked in the first place...
OK, well, I've posted sfc's semantics and view-from-the-hardware*; now
patiently waiting for other NIC vendors to chime in so we can try to
converge on something consistent.
Then again, since they've been CCed since the original patch three weeks
ago, we might be waiting a while :-(
Regarding Ariel Almog's suggested semantics, it seems like they have the
'auto' bit just encoding 'more than one non-auto bit', which is
redundant (i.e. off|rs is always off|rs|auto, whereas rs is never
rs|auto). I don't see how that would be useful.
-Ed
* One complication I left out: we actually have _three_ pairs of sup/req
bits, because we separate 'BaseR for 10G/40G/100G' from 'BaseR for
25G/50G'. I don't know the details of why our HW does this (or why
100G isn't lumped in with the other 25ers) but I think it has to do
with Horrific Ethernet Spec Arcana Man Was Not Meant To Know™.
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: change bridge maintainers
From: David Miller @ 2018-09-28 17:28 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20180927084701.3468-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 27 Sep 2018 10:47:01 +0200
> I haven't been doing reviews only but not active development on bridge
> code for several years. Roopa and Nikolay have been doing most of
> the new features and have agreed to take over as new co-maintainers.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Thanks for all of the years of watching over the bridge code Stephen.
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox