* Re: [PATCH 1/3] drivers/gpu/drm/via: convert put_page() to put_user_page*()
From: John Hubbard @ 2019-07-22 18:53 UTC (permalink / raw)
To: Christoph Hellwig, john.hubbard
Cc: Andrew Morton, Alexander Viro, Björn Töpel,
Boaz Harrosh, Daniel Vetter, Dan Williams, Dave Chinner,
David Airlie, David S . Miller, Ilya Dryomov, Jan Kara,
Jason Gunthorpe, Jens Axboe, Jérôme Glisse,
Johannes Thumshirn, Magnus Karlsson, Matthew Wilcox,
Miklos Szeredi, Ming Lei, Sage Weil, Santosh Shilimkar, Yan Zheng,
netdev, dri-devel, linux-mm, linux-rdma, bpf, LKML
In-Reply-To: <20190722093355.GB29538@lst.de>
On 7/22/19 2:33 AM, Christoph Hellwig wrote:
> On Sun, Jul 21, 2019 at 09:30:10PM -0700, john.hubbard@gmail.com wrote:
>> for (i = 0; i < vsg->num_pages; ++i) {
>> if (NULL != (page = vsg->pages[i])) {
>> if (!PageReserved(page) && (DMA_FROM_DEVICE == vsg->direction))
>> - SetPageDirty(page);
>> - put_page(page);
>> + put_user_pages_dirty(&page, 1);
>> + else
>> + put_user_page(page);
>> }
>
> Can't just pass a dirty argument to put_user_pages? Also do we really
Yes, and in fact that would help a lot more than the single page case,
which is really just cosmetic after all.
> need a separate put_user_page for the single page case?
> put_user_pages_dirty?
Not really. I'm still zeroing in on the ideal API for all these call sites,
and I agree that the approach below is cleaner.
>
> Also the PageReserved check looks bogus, as I can't see how a reserved
> page can end up here. So IMHO the above snippled should really look
> something like this:
>
> put_user_pages(vsg->pages[i], vsg->num_pages,
> vsg->direction == DMA_FROM_DEVICE);
>
> in the end.
>
Agreed.
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply
* [PATCH] net/mlx5: fix -Wtype-limits compilation warnings
From: Qian Cai @ 2019-07-22 18:34 UTC (permalink / raw)
To: saeedm, leonro; +Cc: yishaih, davem, netdev, linux-rdma, linux-kernel, Qian Cai
The commit b9a7ba556207 ("net/mlx5: Use event mask based on device
capabilities") introduced a few compilation warnings due to it bumps
MLX5_EVENT_TYPE_MAX from 0x27 to 0x100 which is always greater than
an "struct {mlx5_eqe|mlx5_nb}.type" that is an "u8".
drivers/net/ethernet/mellanox/mlx5/core/eq.c: In function
'mlx5_eq_notifier_register':
drivers/net/ethernet/mellanox/mlx5/core/eq.c:948:21: warning: comparison
is always false due to limited range of data type [-Wtype-limits]
if (nb->event_type >= MLX5_EVENT_TYPE_MAX)
^~
drivers/net/ethernet/mellanox/mlx5/core/eq.c: In function
'mlx5_eq_notifier_unregister':
drivers/net/ethernet/mellanox/mlx5/core/eq.c:959:21: warning: comparison
is always false due to limited range of data type [-Wtype-limits]
if (nb->event_type >= MLX5_EVENT_TYPE_MAX)
Fix them by removing unnecessary checkings.
Fixes: b9a7ba556207 ("net/mlx5: Use event mask based on device capabilities")
Signed-off-by: Qian Cai <cai@lca.pw>
---
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eq.c b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
index 41f25ea2e8d9..2df9aaa421c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eq.c
@@ -215,11 +215,7 @@ static int mlx5_eq_async_int(struct notifier_block *nb,
*/
dma_rmb();
- if (likely(eqe->type < MLX5_EVENT_TYPE_MAX))
- atomic_notifier_call_chain(&eqt->nh[eqe->type], eqe->type, eqe);
- else
- mlx5_core_warn_once(dev, "notifier_call_chain is not setup for eqe: %d\n", eqe->type);
-
+ atomic_notifier_call_chain(&eqt->nh[eqe->type], eqe->type, eqe);
atomic_notifier_call_chain(&eqt->nh[MLX5_EVENT_TYPE_NOTIFY_ANY], eqe->type, eqe);
++eq->cons_index;
@@ -945,9 +941,6 @@ int mlx5_eq_notifier_register(struct mlx5_core_dev *dev, struct mlx5_nb *nb)
{
struct mlx5_eq_table *eqt = dev->priv.eq_table;
- if (nb->event_type >= MLX5_EVENT_TYPE_MAX)
- return -EINVAL;
-
return atomic_notifier_chain_register(&eqt->nh[nb->event_type], &nb->nb);
}
EXPORT_SYMBOL(mlx5_eq_notifier_register);
@@ -956,9 +949,6 @@ int mlx5_eq_notifier_unregister(struct mlx5_core_dev *dev, struct mlx5_nb *nb)
{
struct mlx5_eq_table *eqt = dev->priv.eq_table;
- if (nb->event_type >= MLX5_EVENT_TYPE_MAX)
- return -EINVAL;
-
return atomic_notifier_chain_unregister(&eqt->nh[nb->event_type], &nb->nb);
}
EXPORT_SYMBOL(mlx5_eq_notifier_unregister);
--
1.8.3.1
^ permalink raw reply related
* [RFC PATCH net-next 12/12] drop_monitor: Add a command to query current configuration
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Users should be able to query the current configuration of drop monitor
before they start using it. Add a command to query the existing
configuration which currently consists of alert mode and packet
truncation length.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/uapi/linux/net_dropmon.h | 2 ++
net/core/drop_monitor.c | 48 ++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/uapi/linux/net_dropmon.h b/include/uapi/linux/net_dropmon.h
index eb36b61485ef..cdcd23a8b72b 100644
--- a/include/uapi/linux/net_dropmon.h
+++ b/include/uapi/linux/net_dropmon.h
@@ -54,6 +54,8 @@ enum {
NET_DM_CMD_START,
NET_DM_CMD_STOP,
NET_DM_CMD_PACKET_ALERT,
+ NET_DM_CMD_CONFIG_GET,
+ NET_DM_CMD_CONFIG_NEW,
_NET_DM_CMD_MAX,
};
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 5af1e1e8d4d0..c753cd6c39ec 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -656,6 +656,50 @@ static int net_dm_cmd_trace(struct sk_buff *skb,
return -EOPNOTSUPP;
}
+static int net_dm_config_fill(struct sk_buff *msg, struct genl_info *info)
+{
+ void *hdr;
+
+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
+ &net_drop_monitor_family, 0, NET_DM_CMD_CONFIG_NEW);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ if (nla_put_u8(msg, NET_DM_ATTR_ALERT_MODE, net_dm_alert_mode))
+ goto nla_put_failure;
+
+ if (nla_put_u32(msg, NET_DM_ATTR_TRUNC_LEN, net_dm_trunc_len))
+ goto nla_put_failure;
+
+ genlmsg_end(msg, hdr);
+
+ return 0;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
+static int net_dm_cmd_config_get(struct sk_buff *skb, struct genl_info *info)
+{
+ struct sk_buff *msg;
+ int rc;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ rc = net_dm_config_fill(msg, info);
+ if (rc)
+ goto err_config_fill;
+
+ return genlmsg_reply(msg, info);
+
+err_config_fill:
+ nlmsg_free(msg);
+ return rc;
+}
+
static int dropmon_net_event(struct notifier_block *ev_block,
unsigned long event, void *ptr)
{
@@ -718,6 +762,10 @@ static const struct genl_ops dropmon_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = net_dm_cmd_trace,
},
+ {
+ .cmd = NET_DM_CMD_CONFIG_GET,
+ .doit = net_dm_cmd_config_get,
+ },
};
static struct genl_family net_drop_monitor_family __ro_after_init = {
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 11/12] drop_monitor: Allow truncation of dropped packets
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
When sending dropped packets to user space it is not always necessary to
copy the entire packet as usually only the headers are of interest.
Allow user to specify the truncation length and add the original length
of the packet as additional metadata to the netlink message.
By default no truncation is performed.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/uapi/linux/net_dropmon.h | 2 ++
net/core/drop_monitor.c | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/include/uapi/linux/net_dropmon.h b/include/uapi/linux/net_dropmon.h
index 7708c8a440a1..eb36b61485ef 100644
--- a/include/uapi/linux/net_dropmon.h
+++ b/include/uapi/linux/net_dropmon.h
@@ -75,6 +75,8 @@ enum net_dm_attr {
NET_DM_ATTR_TIMESTAMP, /* struct timespec */
NET_DM_ATTR_PAYLOAD, /* binary */
NET_DM_ATTR_PAD,
+ NET_DM_ATTR_TRUNC_LEN, /* u32 */
+ NET_DM_ATTR_ORIG_LEN, /* u32 */
__NET_DM_ATTR_MAX,
NET_DM_ATTR_MAX = __NET_DM_ATTR_MAX - 1
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 512935fc235b..5af1e1e8d4d0 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -77,6 +77,7 @@ static unsigned long dm_hw_check_delta = 2*HZ;
static LIST_HEAD(hw_stats_list);
static enum net_dm_alert_mode net_dm_alert_mode = NET_DM_ALERT_MODE_SUMMARY;
+static u32 net_dm_trunc_len;
struct net_dm_skb_cb {
void *pc;
@@ -332,6 +333,8 @@ static size_t net_dm_packet_report_size(size_t payload_len)
nla_total_size(IFNAMSIZ + 1) +
/* NET_DM_ATTR_TIMESTAMP */
nla_total_size(sizeof(struct timespec)) +
+ /* NET_DM_ATTR_ORIG_LEN */
+ nla_total_size(sizeof(u32)) +
/* NET_DM_ATTR_PAYLOAD */
nla_total_size(payload_len);
}
@@ -369,6 +372,9 @@ static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
nla_put(msg, NET_DM_ATTR_TIMESTAMP, sizeof(ts), &ts))
goto nla_put_failure;
+ if (nla_put_u32(msg, NET_DM_ATTR_ORIG_LEN, skb->len))
+ goto nla_put_failure;
+
if (!payload_len)
goto out;
@@ -404,6 +410,8 @@ static void net_dm_packet_report(struct sk_buff *skb)
/* Ensure packet fits inside a single netlink attribute */
payload_len = min_t(size_t, skb->len, NET_DM_MAX_PACKET_SIZE);
+ if (net_dm_trunc_len)
+ payload_len = min_t(size_t, net_dm_trunc_len, payload_len);
msg = nlmsg_new(net_dm_packet_report_size(payload_len), GFP_KERNEL);
if (!msg)
@@ -603,6 +611,16 @@ static int net_dm_alert_mode_set(struct genl_info *info)
return 0;
}
+static int net_dm_trunc_len_set(struct genl_info *info)
+{
+ if (!info->attrs[NET_DM_ATTR_TRUNC_LEN])
+ return 0;
+
+ net_dm_trunc_len = nla_get_u32(info->attrs[NET_DM_ATTR_TRUNC_LEN]);
+
+ return 0;
+}
+
static int net_dm_cmd_config(struct sk_buff *skb,
struct genl_info *info)
{
@@ -618,6 +636,10 @@ static int net_dm_cmd_config(struct sk_buff *skb,
if (rc)
return rc;
+ rc = net_dm_trunc_len_set(info);
+ if (rc)
+ return rc;
+
return 0;
}
@@ -676,6 +698,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
static const struct nla_policy net_dm_nl_policy[NET_DM_ATTR_MAX + 1] = {
[NET_DM_ATTR_UNSPEC] = { .strict_start_type = NET_DM_ATTR_UNSPEC + 1 },
[NET_DM_ATTR_ALERT_MODE] = { .type = NLA_U8 },
+ [NET_DM_ATTR_TRUNC_LEN] = { .type = NLA_U32 },
};
static const struct genl_ops dropmon_ops[] = {
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 10/12] drop_monitor: Add packet alert mode
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
So far drop monitor supported only one alert mode in which a summary of
locations in which packets were recently dropped was sent to user space.
This alert mode is sufficient in order to understand that packets were
dropped, but lacks information to perform a more detailed analysis.
Add a new alert mode in which the dropped packet itself is passed to
user space along with metadata: The drop location (as program counter
and resolved symbol), ingress / egress netdevice and arrival / departure
timestamp. More metadata can be added in the future.
To avoid performing expensive operations in the context in which
kfree_skb() is invoked (can be hard IRQ), the dropped skb is cloned and
queued on per-CPU skb drop list. Then, in process context the netlink
message is allocated, prepared and finally sent to user space.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/uapi/linux/net_dropmon.h | 29 ++++
net/core/drop_monitor.c | 287 ++++++++++++++++++++++++++++++-
2 files changed, 308 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/net_dropmon.h b/include/uapi/linux/net_dropmon.h
index 5edbd0a675fd..7708c8a440a1 100644
--- a/include/uapi/linux/net_dropmon.h
+++ b/include/uapi/linux/net_dropmon.h
@@ -53,6 +53,7 @@ enum {
NET_DM_CMD_CONFIG,
NET_DM_CMD_START,
NET_DM_CMD_STOP,
+ NET_DM_CMD_PACKET_ALERT,
_NET_DM_CMD_MAX,
};
@@ -62,4 +63,32 @@ enum {
* Our group identifiers
*/
#define NET_DM_GRP_ALERT 1
+
+enum net_dm_attr {
+ NET_DM_ATTR_UNSPEC,
+
+ NET_DM_ATTR_ALERT_MODE, /* u8 */
+ NET_DM_ATTR_PC, /* u64 */
+ NET_DM_ATTR_SYMBOL, /* string */
+ NET_DM_ATTR_NETDEV_IFINDEX, /* u32 */
+ NET_DM_ATTR_NETDEV_NAME, /* string */
+ NET_DM_ATTR_TIMESTAMP, /* struct timespec */
+ NET_DM_ATTR_PAYLOAD, /* binary */
+ NET_DM_ATTR_PAD,
+
+ __NET_DM_ATTR_MAX,
+ NET_DM_ATTR_MAX = __NET_DM_ATTR_MAX - 1
+};
+
+/**
+ * enum net_dm_alert_mode - Alert mode.
+ * @NET_DM_ALERT_MODE_SUMMARY: A summary of recent drops is sent to user space.
+ * @NET_DM_ALERT_MODE_PACKET: Each dropped packet is sent to user space along
+ * with metadata.
+ */
+enum net_dm_alert_mode {
+ NET_DM_ALERT_MODE_SUMMARY,
+ NET_DM_ALERT_MODE_PACKET,
+};
+
#endif
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index f441fb653567..512935fc235b 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -54,6 +54,7 @@ static DEFINE_MUTEX(net_dm_mutex);
struct per_cpu_dm_data {
spinlock_t lock; /* Protects 'skb' and 'send_timer' */
struct sk_buff *skb;
+ struct sk_buff_head drop_queue;
struct work_struct dm_alert_work;
struct timer_list send_timer;
};
@@ -75,6 +76,22 @@ static int dm_delay = 1;
static unsigned long dm_hw_check_delta = 2*HZ;
static LIST_HEAD(hw_stats_list);
+static enum net_dm_alert_mode net_dm_alert_mode = NET_DM_ALERT_MODE_SUMMARY;
+
+struct net_dm_skb_cb {
+ void *pc;
+};
+
+#define NET_DM_SKB_CB(__skb) ((struct net_dm_skb_cb *)&((__skb)->cb[0]))
+
+struct net_dm_alert_ops {
+ void (*kfree_skb_probe)(void *ignore, struct sk_buff *skb,
+ void *location);
+ void (*napi_poll_probe)(void *ignore, struct napi_struct *napi,
+ int work, int budget);
+ void (*work_item_func)(struct work_struct *work);
+};
+
static int net_dm_nl_pre_doit(const struct genl_ops *ops,
struct sk_buff *skb, struct genl_info *info)
{
@@ -255,10 +272,194 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi,
rcu_read_unlock();
}
+static const struct net_dm_alert_ops net_dm_alert_summary_ops = {
+ .kfree_skb_probe = trace_kfree_skb_hit,
+ .napi_poll_probe = trace_napi_poll_hit,
+ .work_item_func = send_dm_alert,
+};
+
+static void net_dm_packet_trace_kfree_skb_hit(void *ignore,
+ struct sk_buff *skb,
+ void *location)
+{
+ struct per_cpu_dm_data *data;
+ struct sk_buff *nskb;
+ unsigned long flags;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return;
+
+ NET_DM_SKB_CB(nskb)->pc = location;
+ if (nskb->dev)
+ dev_hold(nskb->dev);
+
+ data = this_cpu_ptr(&dm_cpu_data);
+ spin_lock_irqsave(&data->drop_queue.lock, flags);
+
+ __skb_queue_tail(&data->drop_queue, nskb);
+
+ if (!timer_pending(&data->send_timer)) {
+ data->send_timer.expires = jiffies + dm_delay * HZ;
+ add_timer(&data->send_timer);
+ }
+
+ spin_unlock_irqrestore(&data->drop_queue.lock, flags);
+}
+
+static void net_dm_packet_trace_napi_poll_hit(void *ignore,
+ struct napi_struct *napi,
+ int work, int budget)
+{
+}
+
+#define NET_DM_MAX_SYMBOL_LEN 40
+
+static size_t net_dm_packet_report_size(size_t payload_len)
+{
+ size_t size;
+
+ size = nlmsg_msg_size(GENL_HDRLEN + net_drop_monitor_family.hdrsize);
+
+ return NLMSG_ALIGN(size) +
+ /* NET_DM_ATTR_PC */
+ nla_total_size(sizeof(u64)) +
+ /* NET_DM_ATTR_SYMBOL */
+ nla_total_size(NET_DM_MAX_SYMBOL_LEN + 1) +
+ /* NET_DM_ATTR_NETDEV_IFINDEX */
+ nla_total_size(sizeof(u32)) +
+ /* NET_DM_ATTR_NETDEV_NAME */
+ nla_total_size(IFNAMSIZ + 1) +
+ /* NET_DM_ATTR_TIMESTAMP */
+ nla_total_size(sizeof(struct timespec)) +
+ /* NET_DM_ATTR_PAYLOAD */
+ nla_total_size(payload_len);
+}
+
+static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
+ size_t payload_len)
+{
+ u64 pc = (u64)(uintptr_t) NET_DM_SKB_CB(skb)->pc;
+ char buf[NET_DM_MAX_SYMBOL_LEN];
+ struct nlattr *attr;
+ struct timespec ts;
+ void *hdr;
+
+ hdr = genlmsg_put(msg, 0, 0, &net_drop_monitor_family, 0,
+ NET_DM_CMD_PACKET_ALERT);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ if (nla_put_u64_64bit(msg, NET_DM_ATTR_PC, pc, NET_DM_ATTR_PAD))
+ goto nla_put_failure;
+
+ snprintf(buf, sizeof(buf), "%pS", NET_DM_SKB_CB(skb)->pc);
+ if (nla_put_string(msg, NET_DM_ATTR_SYMBOL, buf))
+ goto nla_put_failure;
+
+ if (skb->dev &&
+ nla_put_u32(msg, NET_DM_ATTR_NETDEV_IFINDEX, skb->dev->ifindex))
+ goto nla_put_failure;
+
+ if (skb->dev &&
+ nla_put_string(msg, NET_DM_ATTR_NETDEV_NAME, skb->dev->name))
+ goto nla_put_failure;
+
+ if (ktime_to_timespec_cond(skb->tstamp, &ts) &&
+ nla_put(msg, NET_DM_ATTR_TIMESTAMP, sizeof(ts), &ts))
+ goto nla_put_failure;
+
+ if (!payload_len)
+ goto out;
+
+ attr = skb_put(msg, nla_total_size(payload_len));
+ attr->nla_type = NET_DM_ATTR_PAYLOAD;
+ attr->nla_len = nla_attr_size(payload_len);
+ if (skb_copy_bits(skb, 0, nla_data(attr), payload_len))
+ goto nla_put_failure;
+
+out:
+ genlmsg_end(msg, hdr);
+
+ return 0;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
+#define NET_DM_MAX_PACKET_SIZE (0xffff - NLA_HDRLEN - NLA_ALIGNTO)
+
+static void net_dm_packet_report(struct sk_buff *skb)
+{
+ struct sk_buff *msg;
+ size_t payload_len;
+ int rc;
+
+ /* Make sure we start copying the packet from the MAC header */
+ if (skb->data > skb_mac_header(skb))
+ skb_push(skb, skb->data - skb_mac_header(skb));
+ else
+ skb_pull(skb, skb_mac_header(skb) - skb->data);
+
+ /* Ensure packet fits inside a single netlink attribute */
+ payload_len = min_t(size_t, skb->len, NET_DM_MAX_PACKET_SIZE);
+
+ msg = nlmsg_new(net_dm_packet_report_size(payload_len), GFP_KERNEL);
+ if (!msg)
+ goto out;
+
+ rc = net_dm_packet_report_fill(msg, skb, payload_len);
+ if (rc) {
+ nlmsg_free(msg);
+ goto out;
+ }
+
+ genlmsg_multicast(&net_drop_monitor_family, msg, 0, 0, GFP_KERNEL);
+
+out:
+ if (skb->dev)
+ dev_put(skb->dev);
+ consume_skb(skb);
+}
+
+static void net_dm_packet_work(struct work_struct *work)
+{
+ struct per_cpu_dm_data *data;
+ struct sk_buff_head list;
+ struct sk_buff *skb;
+ unsigned long flags;
+
+ data = container_of(work, struct per_cpu_dm_data, dm_alert_work);
+
+ __skb_queue_head_init(&list);
+
+ spin_lock_irqsave(&data->drop_queue.lock, flags);
+ skb_queue_splice_tail_init(&data->drop_queue, &list);
+ spin_unlock_irqrestore(&data->drop_queue.lock, flags);
+
+ while ((skb = __skb_dequeue(&list)))
+ net_dm_packet_report(skb);
+}
+
+static const struct net_dm_alert_ops net_dm_alert_packet_ops = {
+ .kfree_skb_probe = net_dm_packet_trace_kfree_skb_hit,
+ .napi_poll_probe = net_dm_packet_trace_napi_poll_hit,
+ .work_item_func = net_dm_packet_work,
+};
+
+static const struct net_dm_alert_ops *net_dm_alert_ops_arr[] = {
+ [NET_DM_ALERT_MODE_SUMMARY] = &net_dm_alert_summary_ops,
+ [NET_DM_ALERT_MODE_PACKET] = &net_dm_alert_packet_ops,
+};
+
static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
{
+ const struct net_dm_alert_ops *ops;
int cpu, rc;
+ ops = net_dm_alert_ops_arr[net_dm_alert_mode];
+
if (!try_module_get(THIS_MODULE)) {
NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
return -ENODEV;
@@ -267,17 +468,17 @@ static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
for_each_possible_cpu(cpu) {
struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
- INIT_WORK(&data->dm_alert_work, send_dm_alert);
+ INIT_WORK(&data->dm_alert_work, ops->work_item_func);
timer_setup(&data->send_timer, sched_send_work, 0);
}
- rc = register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+ rc = register_trace_kfree_skb(ops->kfree_skb_probe, NULL);
if (rc) {
NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to kfree_skb() tracepoint");
goto err_module_put;
}
- rc = register_trace_napi_poll(trace_napi_poll_hit, NULL);
+ rc = register_trace_napi_poll(ops->napi_poll_probe, NULL);
if (rc) {
NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to napi_poll() tracepoint");
goto err_unregister_trace;
@@ -286,7 +487,7 @@ static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
return 0;
err_unregister_trace:
- unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+ unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
err_module_put:
module_put(THIS_MODULE);
return rc;
@@ -295,10 +496,13 @@ static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
static void net_dm_trace_off_set(void)
{
struct dm_hw_stat_delta *new_stat, *temp;
+ const struct net_dm_alert_ops *ops;
int cpu;
- unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
- unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+ ops = net_dm_alert_ops_arr[net_dm_alert_mode];
+
+ unregister_trace_napi_poll(ops->napi_poll_probe, NULL);
+ unregister_trace_kfree_skb(ops->kfree_skb_probe, NULL);
tracepoint_synchronize_unregister();
@@ -307,9 +511,18 @@ static void net_dm_trace_off_set(void)
*/
for_each_possible_cpu(cpu) {
struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
+ struct sk_buff *skb;
del_timer_sync(&data->send_timer);
cancel_work_sync(&data->dm_alert_work);
+ /* If we deactivated a pending timer, some packets are still
+ * queued and we need to free them.
+ */
+ while ((skb = __skb_dequeue(&data->drop_queue))) {
+ if (skb->dev)
+ dev_put(skb->dev);
+ consume_skb(skb);
+ }
}
list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
@@ -351,12 +564,61 @@ static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
return rc;
}
+static int net_dm_alert_mode_get_from_info(struct genl_info *info,
+ enum net_dm_alert_mode *p_alert_mode)
+{
+ u8 val;
+
+ val = nla_get_u8(info->attrs[NET_DM_ATTR_ALERT_MODE]);
+
+ switch (val) {
+ case NET_DM_ALERT_MODE_SUMMARY: /* fall-through */
+ case NET_DM_ALERT_MODE_PACKET:
+ *p_alert_mode = val;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int net_dm_alert_mode_set(struct genl_info *info)
+{
+ struct netlink_ext_ack *extack = info->extack;
+ enum net_dm_alert_mode alert_mode;
+ int rc;
+
+ if (!info->attrs[NET_DM_ATTR_ALERT_MODE])
+ return 0;
+
+ rc = net_dm_alert_mode_get_from_info(info, &alert_mode);
+ if (rc) {
+ NL_SET_ERR_MSG_MOD(extack, "Invalid alert mode");
+ return -EINVAL;
+ }
+
+ net_dm_alert_mode = alert_mode;
+
+ return 0;
+}
+
static int net_dm_cmd_config(struct sk_buff *skb,
struct genl_info *info)
{
- NL_SET_ERR_MSG_MOD(info->extack, "Command not supported");
+ struct netlink_ext_ack *extack = info->extack;
+ int rc;
- return -EOPNOTSUPP;
+ if (trace_state == TRACE_ON) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot configure drop monitor while tracing is on");
+ return -EOPNOTSUPP;
+ }
+
+ rc = net_dm_alert_mode_set(info);
+ if (rc)
+ return rc;
+
+ return 0;
}
static int net_dm_cmd_trace(struct sk_buff *skb,
@@ -411,6 +673,11 @@ static int dropmon_net_event(struct notifier_block *ev_block,
return NOTIFY_DONE;
}
+static const struct nla_policy net_dm_nl_policy[NET_DM_ATTR_MAX + 1] = {
+ [NET_DM_ATTR_UNSPEC] = { .strict_start_type = NET_DM_ATTR_UNSPEC + 1 },
+ [NET_DM_ATTR_ALERT_MODE] = { .type = NLA_U8 },
+};
+
static const struct genl_ops dropmon_ops[] = {
{
.cmd = NET_DM_CMD_CONFIG,
@@ -434,6 +701,8 @@ static struct genl_family net_drop_monitor_family __ro_after_init = {
.hdrsize = 0,
.name = "NET_DM",
.version = 2,
+ .maxattr = NET_DM_ATTR_MAX,
+ .policy = net_dm_nl_policy,
.pre_doit = net_dm_nl_pre_doit,
.post_doit = net_dm_nl_post_doit,
.module = THIS_MODULE,
@@ -479,6 +748,7 @@ static int __init init_net_drop_monitor(void)
INIT_WORK(&data->dm_alert_work, send_dm_alert);
timer_setup(&data->send_timer, sched_send_work, 0);
spin_lock_init(&data->lock);
+ skb_queue_head_init(&data->drop_queue);
reset_per_cpu_data(data);
}
@@ -509,6 +779,7 @@ static void exit_net_drop_monitor(void)
* to this struct and can free the skb inside it
*/
kfree_skb(data->skb);
+ WARN_ON(!skb_queue_empty(&data->drop_queue));
}
BUG_ON(genl_unregister_family(&net_drop_monitor_family));
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 09/12] drop_monitor: Require CAP_NET_ADMIN for drop monitor configuration
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Currently, the configure command does not do anything but return an
error. Subsequent patches will enable the command to change various
configuration options such as alert mode and packet truncation.
Similar to other netlink-based configuration channels, make sure only
users with the CAP_NET_ADMIN capability set can execute this command.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 2f56a61c43c6..f441fb653567 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -416,6 +416,7 @@ static const struct genl_ops dropmon_ops[] = {
.cmd = NET_DM_CMD_CONFIG,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = net_dm_cmd_config,
+ .flags = GENL_ADMIN_PERM,
},
{
.cmd = NET_DM_CMD_START,
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 08/12] drop_monitor: Initialize timer and work item upon tracing enable
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
The timer and work item are currently initialized once during module
init, but subsequent patches will need to associate different functions
with the work item, based on the configured alert mode.
Allow subsequent patches to make that change by initializing and
de-initializing these objects during tracing enable and disable.
This also guarantees that once the request to disable tracing returns,
no more netlink notifications will be generated.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index e68dafaaebcd..2f56a61c43c6 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -257,13 +257,20 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi,
static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
{
- int rc;
+ int cpu, rc;
if (!try_module_get(THIS_MODULE)) {
NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
return -ENODEV;
}
+ for_each_possible_cpu(cpu) {
+ struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
+
+ INIT_WORK(&data->dm_alert_work, send_dm_alert);
+ timer_setup(&data->send_timer, sched_send_work, 0);
+ }
+
rc = register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
if (rc) {
NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to kfree_skb() tracepoint");
@@ -288,12 +295,23 @@ static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
static void net_dm_trace_off_set(void)
{
struct dm_hw_stat_delta *new_stat, *temp;
+ int cpu;
unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
tracepoint_synchronize_unregister();
+ /* Make sure we do not send notifications to user space after request
+ * to stop tracing returns.
+ */
+ for_each_possible_cpu(cpu) {
+ struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu);
+
+ del_timer_sync(&data->send_timer);
+ cancel_work_sync(&data->dm_alert_work);
+ }
+
list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
if (new_stat->dev == NULL) {
list_del_rcu(&new_stat->list);
@@ -481,14 +499,10 @@ static void exit_net_drop_monitor(void)
/*
* Because of the module_get/put we do in the trace state change path
* we are guarnateed not to have any current users when we get here
- * all we need to do is make sure that we don't have any running timers
- * or pending schedule calls
*/
for_each_possible_cpu(cpu) {
data = &per_cpu(dm_cpu_data, cpu);
- del_timer_sync(&data->send_timer);
- cancel_work_sync(&data->dm_alert_work);
/*
* At this point, we should have exclusive access
* to this struct and can free the skb inside it
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 07/12] drop_monitor: Split tracing enable / disable to different functions
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Subsequent patches will need to enable / disable tracing based on the
configured alerting mode.
Reduce the nesting level and prepare for the introduction of this
functionality by splitting the tracing enable / disable operations into
two different functions.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 79 ++++++++++++++++++++++++++---------------
1 file changed, 51 insertions(+), 28 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 099000930736..e68dafaaebcd 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -255,11 +255,58 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi,
rcu_read_unlock();
}
+static int net_dm_trace_on_set(struct netlink_ext_ack *extack)
+{
+ int rc;
+
+ if (!try_module_get(THIS_MODULE)) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
+ return -ENODEV;
+ }
+
+ rc = register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+ if (rc) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to kfree_skb() tracepoint");
+ goto err_module_put;
+ }
+
+ rc = register_trace_napi_poll(trace_napi_poll_hit, NULL);
+ if (rc) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to napi_poll() tracepoint");
+ goto err_unregister_trace;
+ }
+
+ return 0;
+
+err_unregister_trace:
+ unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+err_module_put:
+ module_put(THIS_MODULE);
+ return rc;
+}
+
+static void net_dm_trace_off_set(void)
+{
+ struct dm_hw_stat_delta *new_stat, *temp;
+
+ unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
+ unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
+
+ tracepoint_synchronize_unregister();
+
+ list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
+ if (new_stat->dev == NULL) {
+ list_del_rcu(&new_stat->list);
+ kfree_rcu(new_stat, rcu);
+ }
+ }
+
+ module_put(THIS_MODULE);
+}
+
static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
{
int rc = 0;
- struct dm_hw_stat_delta *new_stat = NULL;
- struct dm_hw_stat_delta *temp;
if (state == trace_state) {
NL_SET_ERR_MSG_MOD(extack, "Trace state already set to requested state");
@@ -268,34 +315,10 @@ static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
switch (state) {
case TRACE_ON:
- if (!try_module_get(THIS_MODULE)) {
- NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
- rc = -ENODEV;
- break;
- }
-
- rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL);
- rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL);
+ rc = net_dm_trace_on_set(extack);
break;
-
case TRACE_OFF:
- rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL);
- rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL);
-
- tracepoint_synchronize_unregister();
-
- /*
- * Clean the device list
- */
- list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
- if (new_stat->dev == NULL) {
- list_del_rcu(&new_stat->list);
- kfree_rcu(new_stat, rcu);
- }
- }
-
- module_put(THIS_MODULE);
-
+ net_dm_trace_off_set();
break;
default:
rc = 1;
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 05/12] drop_monitor: Add extack support
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Add various extack messages to make drop_monitor more user friendly.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 9080e62245b9..1d463c0d4bc5 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -241,7 +241,7 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi,
rcu_read_unlock();
}
-static int set_all_monitor_traces(int state)
+static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
{
int rc = 0;
struct dm_hw_stat_delta *new_stat = NULL;
@@ -250,6 +250,7 @@ static int set_all_monitor_traces(int state)
mutex_lock(&net_dm_mutex);
if (state == trace_state) {
+ NL_SET_ERR_MSG_MOD(extack, "Trace state already set to requested state");
rc = -EAGAIN;
goto out_unlock;
}
@@ -257,6 +258,7 @@ static int set_all_monitor_traces(int state)
switch (state) {
case TRACE_ON:
if (!try_module_get(THIS_MODULE)) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to take reference on module");
rc = -ENODEV;
break;
}
@@ -303,6 +305,8 @@ static int set_all_monitor_traces(int state)
static int net_dm_cmd_config(struct sk_buff *skb,
struct genl_info *info)
{
+ NL_SET_ERR_MSG_MOD(info->extack, "Command not supported");
+
return -EOPNOTSUPP;
}
@@ -311,9 +315,9 @@ static int net_dm_cmd_trace(struct sk_buff *skb,
{
switch (info->genlhdr->cmd) {
case NET_DM_CMD_START:
- return set_all_monitor_traces(TRACE_ON);
+ return set_all_monitor_traces(TRACE_ON, info->extack);
case NET_DM_CMD_STOP:
- return set_all_monitor_traces(TRACE_OFF);
+ return set_all_monitor_traces(TRACE_OFF, info->extack);
}
return -EOPNOTSUPP;
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 06/12] drop_monitor: Use pre_doit / post_doit hooks
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Each operation from user space should be protected by the global drop
monitor mutex. Use the pre_doit / post_doit hooks to take / release the
lock instead of doing it explicitly in each function.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 1d463c0d4bc5..099000930736 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -75,6 +75,20 @@ static int dm_delay = 1;
static unsigned long dm_hw_check_delta = 2*HZ;
static LIST_HEAD(hw_stats_list);
+static int net_dm_nl_pre_doit(const struct genl_ops *ops,
+ struct sk_buff *skb, struct genl_info *info)
+{
+ mutex_lock(&net_dm_mutex);
+
+ return 0;
+}
+
+static void net_dm_nl_post_doit(const struct genl_ops *ops,
+ struct sk_buff *skb, struct genl_info *info)
+{
+ mutex_unlock(&net_dm_mutex);
+}
+
static struct sk_buff *reset_per_cpu_data(struct per_cpu_dm_data *data)
{
size_t al;
@@ -247,12 +261,9 @@ static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
struct dm_hw_stat_delta *new_stat = NULL;
struct dm_hw_stat_delta *temp;
- mutex_lock(&net_dm_mutex);
-
if (state == trace_state) {
NL_SET_ERR_MSG_MOD(extack, "Trace state already set to requested state");
- rc = -EAGAIN;
- goto out_unlock;
+ return -EAGAIN;
}
switch (state) {
@@ -296,9 +307,6 @@ static int set_all_monitor_traces(int state, struct netlink_ext_ack *extack)
else
rc = -EINPROGRESS;
-out_unlock:
- mutex_unlock(&net_dm_mutex);
-
return rc;
}
@@ -384,6 +392,8 @@ static struct genl_family net_drop_monitor_family __ro_after_init = {
.hdrsize = 0,
.name = "NET_DM",
.version = 2,
+ .pre_doit = net_dm_nl_pre_doit,
+ .post_doit = net_dm_nl_post_doit,
.module = THIS_MODULE,
.ops = dropmon_ops,
.n_ops = ARRAY_SIZE(dropmon_ops),
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 04/12] drop_monitor: Avoid multiple blank lines
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Remove multiple blank lines which are visually annoying and useless.
This suppresses the "Please don't use multiple blank lines" checkpatch
messages.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 35d31b007da4..9080e62245b9 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -300,7 +300,6 @@ static int set_all_monitor_traces(int state)
return rc;
}
-
static int net_dm_cmd_config(struct sk_buff *skb,
struct genl_info *info)
{
@@ -427,7 +426,6 @@ static int __init init_net_drop_monitor(void)
reset_per_cpu_data(data);
}
-
goto out;
out_unreg:
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 03/12] drop_monitor: Document scope of spinlock
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
While 'per_cpu_dm_data' is a per-CPU variable, its 'skb' and
'send_timer' fields can be accessed concurrently by the CPU sending the
netlink notification to user space from the workqueue and the CPU
tracing kfree_skb(). This spinlock is meant to protect against that.
Document its scope and suppress the checkpatch message "spinlock_t
definition without comment".
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 000ec8b66d1e..35d31b007da4 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -52,7 +52,7 @@ static int trace_state = TRACE_OFF;
static DEFINE_MUTEX(net_dm_mutex);
struct per_cpu_dm_data {
- spinlock_t lock;
+ spinlock_t lock; /* Protects 'skb' and 'send_timer' */
struct sk_buff *skb;
struct work_struct dm_alert_work;
struct timer_list send_timer;
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 00/12] drop_monitor: Capture dropped packets and metadata
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
So far drop monitor supported only one mode of operation in which a
summary of recent packet drops is periodically sent to user space as a
netlink event. The event only includes the drop location (program
counter) and number of drops in the last interval.
While this mode of operation allows one to understand if the system is
dropping packets, it is not sufficient if a more detailed analysis is
required. Both the packet itself and related metadata are missing.
This patchset extends drop monitor with another mode of operation where
the packet - potentially truncated - and metadata (e.g., drop location,
timestamp, netdev) are sent to user space as a netlink event. Thanks to
the extensible nature of netlink, more metadata can be added in the
future.
To avoid performing expensive operations in the context in which
kfree_skb() is called, the dropped skbs are cloned and queued on per-CPU
skb drop list. The list is then processed in process context (using a
workqueue), where the netlink messages are allocated, prepared and
finally sent to user space.
As a follow-up, I plan to integrate drop monitor with devlink and allow
the latter to call into drop monitor to report hardware drops. In the
future, XDP drops can be added as well, thereby making drop monitor the
go-to netlink channel for diagnosing all packet drops.
Example usage with patched dropwatch [1] can be found here [2]. Example
dissection of drop monitor netlink events with patched wireshark [3] can
be found here [4]. I will submit both changes upstream after the kernel
changes are accepted.
Patches #1-#6 are just cleanups with no functional changes intended.
Patches #7-#8 perform small refactoring before the actual changes are
introduced in the last four patches.
[1] https://github.com/idosch/dropwatch/tree/packet-mode
[2] https://gist.github.com/idosch/7391b77da0b16182406189561fdfa1ef#file-gistfile1-txt
[3] https://github.com/idosch/wireshark/tree/drop-monitor
[4] https://gist.github.com/idosch/7391b77da0b16182406189561fdfa1ef#file-gistfile2-txt
Ido Schimmel (12):
drop_monitor: Use correct error code
drop_monitor: Rename and document scope of mutex
drop_monitor: Document scope of spinlock
drop_monitor: Avoid multiple blank lines
drop_monitor: Add extack support
drop_monitor: Use pre_doit / post_doit hooks
drop_monitor: Split tracing enable / disable to different functions
drop_monitor: Initialize timer and work item upon tracing enable
drop_monitor: Require CAP_NET_ADMIN for drop monitor configuration
drop_monitor: Add packet alert mode
drop_monitor: Allow truncation of dropped packets
drop_monitor: Add a command to query current configuration
include/uapi/linux/net_dropmon.h | 33 +++
net/core/drop_monitor.c | 492 ++++++++++++++++++++++++++++---
2 files changed, 478 insertions(+), 47 deletions(-)
--
2.21.0
^ permalink raw reply
* [RFC PATCH net-next 02/12] drop_monitor: Rename and document scope of mutex
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
The 'trace_state_mutex' does not only protect the global 'trace_state'
variable, but also the global 'hw_stats_list'.
Subsequent patches are going add more operations from user space to
drop_monitor and these all need to be mutually exclusive.
Rename 'trace_state_mutex' to the more fitting 'net_dm_mutex' name and
document its scope.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index dcb4d2aeb2a8..000ec8b66d1e 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -43,7 +43,13 @@
* netlink alerts
*/
static int trace_state = TRACE_OFF;
-static DEFINE_MUTEX(trace_state_mutex);
+
+/* net_dm_mutex
+ *
+ * An overall lock guarding every operation coming from userspace.
+ * It also guards the global 'hw_stats_list' list.
+ */
+static DEFINE_MUTEX(net_dm_mutex);
struct per_cpu_dm_data {
spinlock_t lock;
@@ -241,7 +247,7 @@ static int set_all_monitor_traces(int state)
struct dm_hw_stat_delta *new_stat = NULL;
struct dm_hw_stat_delta *temp;
- mutex_lock(&trace_state_mutex);
+ mutex_lock(&net_dm_mutex);
if (state == trace_state) {
rc = -EAGAIN;
@@ -289,7 +295,7 @@ static int set_all_monitor_traces(int state)
rc = -EINPROGRESS;
out_unlock:
- mutex_unlock(&trace_state_mutex);
+ mutex_unlock(&net_dm_mutex);
return rc;
}
@@ -330,12 +336,12 @@ static int dropmon_net_event(struct notifier_block *ev_block,
new_stat->dev = dev;
new_stat->last_rx = jiffies;
- mutex_lock(&trace_state_mutex);
+ mutex_lock(&net_dm_mutex);
list_add_rcu(&new_stat->list, &hw_stats_list);
- mutex_unlock(&trace_state_mutex);
+ mutex_unlock(&net_dm_mutex);
break;
case NETDEV_UNREGISTER:
- mutex_lock(&trace_state_mutex);
+ mutex_lock(&net_dm_mutex);
list_for_each_entry_safe(new_stat, tmp, &hw_stats_list, list) {
if (new_stat->dev == dev) {
new_stat->dev = NULL;
@@ -346,7 +352,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
}
}
}
- mutex_unlock(&trace_state_mutex);
+ mutex_unlock(&net_dm_mutex);
break;
}
out:
--
2.21.0
^ permalink raw reply related
* [RFC PATCH net-next 01/12] drop_monitor: Use correct error code
From: Ido Schimmel @ 2019-07-22 18:31 UTC (permalink / raw)
To: netdev
Cc: davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski, toke,
andy, f.fainelli, andrew, vivien.didelot, mlxsw, Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
The error code 'ENOTSUPP' is reserved for use with NFS. Use 'EOPNOTSUPP'
instead.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
net/core/drop_monitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 4ea4347f5062..dcb4d2aeb2a8 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -298,7 +298,7 @@ static int set_all_monitor_traces(int state)
static int net_dm_cmd_config(struct sk_buff *skb,
struct genl_info *info)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static int net_dm_cmd_trace(struct sk_buff *skb,
@@ -311,7 +311,7 @@ static int net_dm_cmd_trace(struct sk_buff *skb,
return set_all_monitor_traces(TRACE_OFF);
}
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static int dropmon_net_event(struct notifier_block *ev_block,
--
2.21.0
^ permalink raw reply related
* Re: [PATCH iproute2] etf: make printing of variable JSON friendly
From: David Ahern @ 2019-07-22 18:21 UTC (permalink / raw)
To: Vedang Patel, netdev
Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
leandro.maciel.dorileo
In-Reply-To: <1563572443-10879-1-git-send-email-vedang.patel@intel.com>
On 7/19/19 3:40 PM, Vedang Patel wrote:
> In iproute2 txtime-assist series, it was pointed out that print_bool()
> should be used to print binary values. This is to make it JSON friendly.
>
> So, make the corresponding changes in ETF.
>
> Fixes: 8ccd49383cdc ("etf: Add skip_sock_check")
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
> ---
> tc/q_etf.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tc/q_etf.c b/tc/q_etf.c
> index c2090589bc64..307c50eed48b 100644
> --- a/tc/q_etf.c
> +++ b/tc/q_etf.c
> @@ -176,12 +176,12 @@ static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> get_clock_name(qopt->clockid));
>
> print_uint(PRINT_ANY, "delta", "delta %d ", qopt->delta);
> - print_string(PRINT_ANY, "offload", "offload %s ",
> - (qopt->flags & TC_ETF_OFFLOAD_ON) ? "on" : "off");
> - print_string(PRINT_ANY, "deadline_mode", "deadline_mode %s ",
> - (qopt->flags & TC_ETF_DEADLINE_MODE_ON) ? "on" : "off");
> - print_string(PRINT_ANY, "skip_sock_check", "skip_sock_check %s",
> - (qopt->flags & TC_ETF_SKIP_SOCK_CHECK) ? "on" : "off");
> + if (qopt->flags & TC_ETF_OFFLOAD_ON)
> + print_bool(PRINT_ANY, "offload", "offload ", true);
> + if (qopt->flags & TC_ETF_DEADLINE_MODE_ON)
> + print_bool(PRINT_ANY, "deadline_mode", "deadline_mode ", true);
> + if (qopt->flags & TC_ETF_SKIP_SOCK_CHECK)
> + print_bool(PRINT_ANY, "skip_sock_check", "skip_sock_check", true);
>
> return 0;
> }
>
This changes existing output for TC_ETF_OFFLOAD_ON and
TC_ETF_DEADLINE_MODE_ON which were added a year ago.
^ permalink raw reply
* [vhost:linux-next 4/5] kernel/rcu/tiny.c:138:22: error: 'rcu_data' undeclared
From: kbuild test robot @ 2019-07-22 17:41 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kbuild-all, kvm, virtualization, netdev
[-- Attachment #1: Type: text/plain, Size: 3469 bytes --]
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/mst/vhost.git linux-next
head: 25da3c2b439666602852820e3231349085682e1a
commit: 4cfd64ce2ad979cbd9a97e1500533d2f5f1355b8 [4/5] rcu: add count of outstanding callbacks
config: i386-allnoconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
git checkout 4cfd64ce2ad979cbd9a97e1500533d2f5f1355b8
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from include/asm-generic/percpu.h:7:0,
from arch/x86/include/asm/percpu.h:556,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/wait.h:9,
from include/linux/completion.h:12,
from kernel/rcu/tiny.c:12:
kernel/rcu/tiny.c: In function 'call_rcu_outstanding':
>> kernel/rcu/tiny.c:138:22: error: 'rcu_data' undeclared (first use in this function)
rdp = this_cpu_ptr(&rcu_data);
^
include/linux/percpu-defs.h:220:47: note: in definition of macro '__verify_pcpu_ptr'
const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL; \
^~~
>> include/linux/percpu-defs.h:264:47: note: in expansion of macro 'VERIFY_PERCPU_PTR'
#define per_cpu_ptr(ptr, cpu) ({ (void)(cpu); VERIFY_PERCPU_PTR(ptr); })
^~~~~~~~~~~~~~~~~
>> include/linux/percpu-defs.h:265:26: note: in expansion of macro 'per_cpu_ptr'
#define raw_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
^~~~~~~~~~~
>> include/linux/percpu-defs.h:266:27: note: in expansion of macro 'raw_cpu_ptr'
#define this_cpu_ptr(ptr) raw_cpu_ptr(ptr)
^~~~~~~~~~~
>> kernel/rcu/tiny.c:138:8: note: in expansion of macro 'this_cpu_ptr'
rdp = this_cpu_ptr(&rcu_data);
^~~~~~~~~~~~
kernel/rcu/tiny.c:138:22: note: each undeclared identifier is reported only once for each function it appears in
rdp = this_cpu_ptr(&rcu_data);
^
include/linux/percpu-defs.h:220:47: note: in definition of macro '__verify_pcpu_ptr'
const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL; \
^~~
>> include/linux/percpu-defs.h:264:47: note: in expansion of macro 'VERIFY_PERCPU_PTR'
#define per_cpu_ptr(ptr, cpu) ({ (void)(cpu); VERIFY_PERCPU_PTR(ptr); })
^~~~~~~~~~~~~~~~~
>> include/linux/percpu-defs.h:265:26: note: in expansion of macro 'per_cpu_ptr'
#define raw_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
^~~~~~~~~~~
>> include/linux/percpu-defs.h:266:27: note: in expansion of macro 'raw_cpu_ptr'
#define this_cpu_ptr(ptr) raw_cpu_ptr(ptr)
^~~~~~~~~~~
>> kernel/rcu/tiny.c:138:8: note: in expansion of macro 'this_cpu_ptr'
rdp = this_cpu_ptr(&rcu_data);
^~~~~~~~~~~~
vim +/rcu_data +138 kernel/rcu/tiny.c
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7283 bytes --]
^ permalink raw reply
* [PATCH net-next 4/4] sctp: factor out sctp_connect_add_peer
From: Xin Long @ 2019-07-22 17:38 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1563817029.git.lucien.xin@gmail.com>
In this function factored out from sctp_sendmsg_new_asoc() and
__sctp_connect(), it adds a peer with the other addr into the
asoc after this asoc is created with the 1st addr.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 76 +++++++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 45 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 420abdb..6584c19 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1111,6 +1111,33 @@ static int sctp_connect_new_asoc(struct sctp_endpoint *ep,
return err;
}
+static int sctp_connect_add_peer(struct sctp_association *asoc,
+ union sctp_addr *daddr, int addr_len)
+{
+ struct sctp_endpoint *ep = asoc->ep;
+ struct sctp_association *old;
+ struct sctp_transport *t;
+ int err;
+
+ err = sctp_verify_addr(ep->base.sk, daddr, addr_len);
+ if (err)
+ return err;
+
+ old = sctp_endpoint_lookup_assoc(ep, daddr, &t);
+ if (old && old != asoc)
+ return old->state >= SCTP_STATE_ESTABLISHED ? -EISCONN
+ : -EALREADY;
+
+ if (sctp_endpoint_is_peeled_off(ep, daddr))
+ return -EADDRNOTAVAIL;
+
+ t = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
+ if (!t)
+ return -ENOMEM;
+
+ return 0;
+}
+
/* __sctp_connect(struct sock* sk, struct sockaddr *kaddrs, int addrs_size)
*
* Common routine for handling connect() and sctp_connectx().
@@ -1119,10 +1146,10 @@ static int sctp_connect_new_asoc(struct sctp_endpoint *ep,
static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
int addrs_size, int flags, sctp_assoc_t *assoc_id)
{
- struct sctp_association *old, *asoc;
struct sctp_sock *sp = sctp_sk(sk);
struct sctp_endpoint *ep = sp->ep;
struct sctp_transport *transport;
+ struct sctp_association *asoc;
int addrcnt, walk_size, err;
void *addr_buf = kaddrs;
union sctp_addr *daddr;
@@ -1168,29 +1195,10 @@ static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
if (asoc->peer.port != ntohs(daddr->v4.sin_port))
goto out_free;
- err = sctp_verify_addr(sk, daddr, af->sockaddr_len);
+ err = sctp_connect_add_peer(asoc, daddr, af->sockaddr_len);
if (err)
goto out_free;
- old = sctp_endpoint_lookup_assoc(ep, daddr, &transport);
- if (old && old != asoc) {
- err = old->state >= SCTP_STATE_ESTABLISHED ? -EISCONN
- : -EALREADY;
- goto out_free;
- }
-
- if (sctp_endpoint_is_peeled_off(ep, daddr)) {
- err = -EADDRNOTAVAIL;
- goto out_free;
- }
-
- transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL,
- SCTP_UNKNOWN);
- if (!transport) {
- err = -ENOMEM;
- goto out_free;
- }
-
addr_buf += af->sockaddr_len;
walk_size += af->sockaddr_len;
addrcnt++;
@@ -1684,8 +1692,6 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
/* sendv addr list parse */
for_each_cmsghdr(cmsg, cmsgs->addrs_msg) {
- struct sctp_transport *transport;
- struct sctp_association *old;
union sctp_addr _daddr;
int dlen;
@@ -1719,30 +1725,10 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
daddr->v6.sin6_port = htons(asoc->peer.port);
memcpy(&daddr->v6.sin6_addr, CMSG_DATA(cmsg), dlen);
}
- err = sctp_verify_addr(sk, daddr, sizeof(*daddr));
- if (err)
- goto free;
-
- old = sctp_endpoint_lookup_assoc(ep, daddr, &transport);
- if (old && old != asoc) {
- if (old->state >= SCTP_STATE_ESTABLISHED)
- err = -EISCONN;
- else
- err = -EALREADY;
- goto free;
- }
- if (sctp_endpoint_is_peeled_off(ep, daddr)) {
- err = -EADDRNOTAVAIL;
- goto free;
- }
-
- transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL,
- SCTP_UNKNOWN);
- if (!transport) {
- err = -ENOMEM;
+ err = sctp_connect_add_peer(asoc, daddr, sizeof(*daddr));
+ if (err)
goto free;
- }
}
return 0;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 3/4] sctp: factor out sctp_connect_new_asoc
From: Xin Long @ 2019-07-22 17:37 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1563817029.git.lucien.xin@gmail.com>
In this function factored out from sctp_sendmsg_new_asoc() and
__sctp_connect(), it creates the asoc and adds a peer with the
1st addr.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 160 ++++++++++++++++++++++++++----------------------------
1 file changed, 76 insertions(+), 84 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 49837e9..420abdb 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1044,6 +1044,73 @@ static int sctp_setsockopt_bindx(struct sock *sk,
return err;
}
+static int sctp_connect_new_asoc(struct sctp_endpoint *ep,
+ const union sctp_addr *daddr,
+ const struct sctp_initmsg *init,
+ struct sctp_transport **tp)
+{
+ struct sctp_association *asoc;
+ struct sock *sk = ep->base.sk;
+ struct net *net = sock_net(sk);
+ enum sctp_scope scope;
+ int err;
+
+ if (sctp_endpoint_is_peeled_off(ep, daddr))
+ return -EADDRNOTAVAIL;
+
+ if (!ep->base.bind_addr.port) {
+ if (sctp_autobind(sk))
+ return -EAGAIN;
+ } else {
+ if (ep->base.bind_addr.port < inet_prot_sock(net) &&
+ !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE))
+ return -EACCES;
+ }
+
+ scope = sctp_scope(daddr);
+ asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
+ if (!asoc)
+ return -ENOMEM;
+
+ err = sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL);
+ if (err < 0)
+ goto free;
+
+ *tp = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
+ if (!*tp) {
+ err = -ENOMEM;
+ goto free;
+ }
+
+ if (!init)
+ return 0;
+
+ if (init->sinit_num_ostreams) {
+ __u16 outcnt = init->sinit_num_ostreams;
+
+ asoc->c.sinit_num_ostreams = outcnt;
+ /* outcnt has been changed, need to re-init stream */
+ err = sctp_stream_init(&asoc->stream, outcnt, 0, GFP_KERNEL);
+ if (err)
+ goto free;
+ }
+
+ if (init->sinit_max_instreams)
+ asoc->c.sinit_max_instreams = init->sinit_max_instreams;
+
+ if (init->sinit_max_attempts)
+ asoc->max_init_attempts = init->sinit_max_attempts;
+
+ if (init->sinit_max_init_timeo)
+ asoc->max_init_timeo =
+ msecs_to_jiffies(init->sinit_max_init_timeo);
+
+ return 0;
+free:
+ sctp_association_free(asoc);
+ return err;
+}
+
/* __sctp_connect(struct sock* sk, struct sockaddr *kaddrs, int addrs_size)
*
* Common routine for handling connect() and sctp_connectx().
@@ -1056,11 +1123,9 @@ static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
struct sctp_sock *sp = sctp_sk(sk);
struct sctp_endpoint *ep = sp->ep;
struct sctp_transport *transport;
- struct net *net = sock_net(sk);
int addrcnt, walk_size, err;
void *addr_buf = kaddrs;
union sctp_addr *daddr;
- enum sctp_scope scope;
struct sctp_af *af;
long timeo;
@@ -1082,32 +1147,10 @@ static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
return asoc->state >= SCTP_STATE_ESTABLISHED ? -EISCONN
: -EALREADY;
- if (sctp_endpoint_is_peeled_off(ep, daddr))
- return -EADDRNOTAVAIL;
-
- if (!ep->base.bind_addr.port) {
- if (sctp_autobind(sk))
- return -EAGAIN;
- } else {
- if (ep->base.bind_addr.port < inet_prot_sock(net) &&
- !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE))
- return -EACCES;
- }
-
- scope = sctp_scope(daddr);
- asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
- if (!asoc)
- return -ENOMEM;
-
- err = sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL);
- if (err < 0)
- goto out_free;
-
- transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
- if (!transport) {
- err = -ENOMEM;
- goto out_free;
- }
+ err = sctp_connect_new_asoc(ep, daddr, NULL, &transport);
+ if (err)
+ return err;
+ asoc = transport->asoc;
addr_buf += af->sockaddr_len;
walk_size = af->sockaddr_len;
@@ -1162,7 +1205,7 @@ static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
goto out_free;
}
- err = sctp_primitive_ASSOCIATE(net, asoc, NULL);
+ err = sctp_primitive_ASSOCIATE(sock_net(sk), asoc, NULL);
if (err < 0)
goto out_free;
@@ -1598,9 +1641,7 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
struct sctp_transport **tp)
{
struct sctp_endpoint *ep = sctp_sk(sk)->ep;
- struct net *net = sock_net(sk);
struct sctp_association *asoc;
- enum sctp_scope scope;
struct cmsghdr *cmsg;
__be32 flowinfo = 0;
struct sctp_af *af;
@@ -1615,20 +1656,6 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
sctp_sstate(sk, CLOSING)))
return -EADDRNOTAVAIL;
- if (sctp_endpoint_is_peeled_off(ep, daddr))
- return -EADDRNOTAVAIL;
-
- if (!ep->base.bind_addr.port) {
- if (sctp_autobind(sk))
- return -EAGAIN;
- } else {
- if (ep->base.bind_addr.port < inet_prot_sock(net) &&
- !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE))
- return -EACCES;
- }
-
- scope = sctp_scope(daddr);
-
/* Label connection socket for first association 1-to-many
* style for client sequence socket()->sendmsg(). This
* needs to be done before sctp_assoc_add_peer() as that will
@@ -1644,45 +1671,10 @@ static int sctp_sendmsg_new_asoc(struct sock *sk, __u16 sflags,
if (err < 0)
return err;
- asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
- if (!asoc)
- return -ENOMEM;
-
- if (sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL) < 0) {
- err = -ENOMEM;
- goto free;
- }
-
- if (cmsgs->init) {
- struct sctp_initmsg *init = cmsgs->init;
-
- if (init->sinit_num_ostreams) {
- __u16 outcnt = init->sinit_num_ostreams;
-
- asoc->c.sinit_num_ostreams = outcnt;
- /* outcnt has been changed, need to re-init stream */
- err = sctp_stream_init(&asoc->stream, outcnt, 0,
- GFP_KERNEL);
- if (err)
- goto free;
- }
-
- if (init->sinit_max_instreams)
- asoc->c.sinit_max_instreams = init->sinit_max_instreams;
-
- if (init->sinit_max_attempts)
- asoc->max_init_attempts = init->sinit_max_attempts;
-
- if (init->sinit_max_init_timeo)
- asoc->max_init_timeo =
- msecs_to_jiffies(init->sinit_max_init_timeo);
- }
-
- *tp = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
- if (!*tp) {
- err = -ENOMEM;
- goto free;
- }
+ err = sctp_connect_new_asoc(ep, daddr, cmsgs->init, tp);
+ if (err)
+ return err;
+ asoc = (*tp)->asoc;
if (!cmsgs->addrs_msg)
return 0;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 2/4] sctp: clean up __sctp_connect
From: Xin Long @ 2019-07-22 17:37 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1563817029.git.lucien.xin@gmail.com>
__sctp_connect is doing quit similar things as sctp_sendmsg_new_asoc.
To factor out common functions, this patch is to clean up their code
to make them look more similar:
1. create the asoc and add a peer with the 1st addr.
2. add peers with the other addrs into this asoc one by one.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 211 +++++++++++++++++++-----------------------------------
1 file changed, 75 insertions(+), 136 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 5f92e4a..49837e9 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1049,154 +1049,108 @@ static int sctp_setsockopt_bindx(struct sock *sk,
* Common routine for handling connect() and sctp_connectx().
* Connect will come in with just a single address.
*/
-static int __sctp_connect(struct sock *sk,
- struct sockaddr *kaddrs,
- int addrs_size, int flags,
- sctp_assoc_t *assoc_id)
+static int __sctp_connect(struct sock *sk, struct sockaddr *kaddrs,
+ int addrs_size, int flags, sctp_assoc_t *assoc_id)
{
- struct net *net = sock_net(sk);
- struct sctp_sock *sp;
- struct sctp_endpoint *ep;
- struct sctp_association *asoc = NULL;
- struct sctp_association *asoc2;
+ struct sctp_association *old, *asoc;
+ struct sctp_sock *sp = sctp_sk(sk);
+ struct sctp_endpoint *ep = sp->ep;
struct sctp_transport *transport;
- union sctp_addr to;
+ struct net *net = sock_net(sk);
+ int addrcnt, walk_size, err;
+ void *addr_buf = kaddrs;
+ union sctp_addr *daddr;
enum sctp_scope scope;
+ struct sctp_af *af;
long timeo;
- int err = 0;
- int addrcnt = 0;
- int walk_size = 0;
- union sctp_addr *sa_addr = NULL;
- void *addr_buf;
- unsigned short port;
- sp = sctp_sk(sk);
- ep = sp->ep;
-
- /* connect() cannot be done on a socket that is already in ESTABLISHED
- * state - UDP-style peeled off socket or a TCP-style socket that
- * is already connected.
- * It cannot be done even on a TCP-style listening socket.
- */
if (sctp_sstate(sk, ESTABLISHED) || sctp_sstate(sk, CLOSING) ||
- (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING))) {
- err = -EISCONN;
- goto out_free;
+ (sctp_style(sk, TCP) && sctp_sstate(sk, LISTENING)))
+ return -EISCONN;
+
+ daddr = addr_buf;
+ af = sctp_get_af_specific(daddr->sa.sa_family);
+ if (!af || af->sockaddr_len > addrs_size)
+ return -EINVAL;
+
+ err = sctp_verify_addr(sk, daddr, af->sockaddr_len);
+ if (err)
+ return err;
+
+ asoc = sctp_endpoint_lookup_assoc(ep, daddr, &transport);
+ if (asoc)
+ return asoc->state >= SCTP_STATE_ESTABLISHED ? -EISCONN
+ : -EALREADY;
+
+ if (sctp_endpoint_is_peeled_off(ep, daddr))
+ return -EADDRNOTAVAIL;
+
+ if (!ep->base.bind_addr.port) {
+ if (sctp_autobind(sk))
+ return -EAGAIN;
+ } else {
+ if (ep->base.bind_addr.port < inet_prot_sock(net) &&
+ !ns_capable(net->user_ns, CAP_NET_BIND_SERVICE))
+ return -EACCES;
}
- /* Walk through the addrs buffer and count the number of addresses. */
- addr_buf = kaddrs;
- while (walk_size < addrs_size) {
- struct sctp_af *af;
+ scope = sctp_scope(daddr);
+ asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
+ if (!asoc)
+ return -ENOMEM;
- if (walk_size + sizeof(sa_family_t) > addrs_size) {
- err = -EINVAL;
- goto out_free;
- }
+ err = sctp_assoc_set_bind_addr_from_ep(asoc, scope, GFP_KERNEL);
+ if (err < 0)
+ goto out_free;
- sa_addr = addr_buf;
- af = sctp_get_af_specific(sa_addr->sa.sa_family);
+ transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL, SCTP_UNKNOWN);
+ if (!transport) {
+ err = -ENOMEM;
+ goto out_free;
+ }
- /* If the address family is not supported or if this address
- * causes the address buffer to overflow return EINVAL.
- */
- if (!af || (walk_size + af->sockaddr_len) > addrs_size) {
- err = -EINVAL;
+ addr_buf += af->sockaddr_len;
+ walk_size = af->sockaddr_len;
+ addrcnt = 1;
+ while (walk_size < addrs_size) {
+ err = -EINVAL;
+ if (walk_size + sizeof(sa_family_t) > addrs_size)
goto out_free;
- }
-
- port = ntohs(sa_addr->v4.sin_port);
- /* Save current address so we can work with it */
- memcpy(&to, sa_addr, af->sockaddr_len);
+ daddr = addr_buf;
+ af = sctp_get_af_specific(daddr->sa.sa_family);
+ if (!af || af->sockaddr_len + walk_size > addrs_size)
+ goto out_free;
- err = sctp_verify_addr(sk, &to, af->sockaddr_len);
- if (err)
+ if (asoc->peer.port != ntohs(daddr->v4.sin_port))
goto out_free;
- /* Make sure the destination port is correctly set
- * in all addresses.
- */
- if (asoc && asoc->peer.port && asoc->peer.port != port) {
- err = -EINVAL;
+ err = sctp_verify_addr(sk, daddr, af->sockaddr_len);
+ if (err)
goto out_free;
- }
- /* Check if there already is a matching association on the
- * endpoint (other than the one created here).
- */
- asoc2 = sctp_endpoint_lookup_assoc(ep, &to, &transport);
- if (asoc2 && asoc2 != asoc) {
- if (asoc2->state >= SCTP_STATE_ESTABLISHED)
- err = -EISCONN;
- else
- err = -EALREADY;
+ old = sctp_endpoint_lookup_assoc(ep, daddr, &transport);
+ if (old && old != asoc) {
+ err = old->state >= SCTP_STATE_ESTABLISHED ? -EISCONN
+ : -EALREADY;
goto out_free;
}
- /* If we could not find a matching association on the endpoint,
- * make sure that there is no peeled-off association matching
- * the peer address even on another socket.
- */
- if (sctp_endpoint_is_peeled_off(ep, &to)) {
+ if (sctp_endpoint_is_peeled_off(ep, daddr)) {
err = -EADDRNOTAVAIL;
goto out_free;
}
- if (!asoc) {
- /* If a bind() or sctp_bindx() is not called prior to
- * an sctp_connectx() call, the system picks an
- * ephemeral port and will choose an address set
- * equivalent to binding with a wildcard address.
- */
- if (!ep->base.bind_addr.port) {
- if (sctp_autobind(sk)) {
- err = -EAGAIN;
- goto out_free;
- }
- } else {
- /*
- * If an unprivileged user inherits a 1-many
- * style socket with open associations on a
- * privileged port, it MAY be permitted to
- * accept new associations, but it SHOULD NOT
- * be permitted to open new associations.
- */
- if (ep->base.bind_addr.port <
- inet_prot_sock(net) &&
- !ns_capable(net->user_ns,
- CAP_NET_BIND_SERVICE)) {
- err = -EACCES;
- goto out_free;
- }
- }
-
- scope = sctp_scope(&to);
- asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
- if (!asoc) {
- err = -ENOMEM;
- goto out_free;
- }
-
- err = sctp_assoc_set_bind_addr_from_ep(asoc, scope,
- GFP_KERNEL);
- if (err < 0) {
- goto out_free;
- }
-
- }
-
- /* Prime the peer's transport structures. */
- transport = sctp_assoc_add_peer(asoc, &to, GFP_KERNEL,
+ transport = sctp_assoc_add_peer(asoc, daddr, GFP_KERNEL,
SCTP_UNKNOWN);
if (!transport) {
err = -ENOMEM;
goto out_free;
}
- addrcnt++;
- addr_buf += af->sockaddr_len;
+ addr_buf += af->sockaddr_len;
walk_size += af->sockaddr_len;
+ addrcnt++;
}
/* In case the user of sctp_connectx() wants an association
@@ -1209,39 +1163,24 @@ static int __sctp_connect(struct sock *sk,
}
err = sctp_primitive_ASSOCIATE(net, asoc, NULL);
- if (err < 0) {
+ if (err < 0)
goto out_free;
- }
/* Initialize sk's dport and daddr for getpeername() */
inet_sk(sk)->inet_dport = htons(asoc->peer.port);
- sp->pf->to_sk_daddr(sa_addr, sk);
+ sp->pf->to_sk_daddr(daddr, sk);
sk->sk_err = 0;
- timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
-
if (assoc_id)
*assoc_id = asoc->assoc_id;
- err = sctp_wait_for_connect(asoc, &timeo);
- /* Note: the asoc may be freed after the return of
- * sctp_wait_for_connect.
- */
-
- /* Don't free association on exit. */
- asoc = NULL;
+ timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+ return sctp_wait_for_connect(asoc, &timeo);
out_free:
pr_debug("%s: took out_free path with asoc:%p kaddrs:%p err:%d\n",
__func__, asoc, kaddrs, err);
-
- if (asoc) {
- /* sctp_primitive_ASSOCIATE may have added this association
- * To the hash table, try to unhash it, just in case, its a noop
- * if it wasn't hashed so we're safe
- */
- sctp_association_free(asoc);
- }
+ sctp_association_free(asoc);
return err;
}
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 1/4] sctp: check addr_size with sa_family_t size in __sctp_setsockopt_connectx
From: Xin Long @ 2019-07-22 17:37 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1563817029.git.lucien.xin@gmail.com>
Now __sctp_connect() is called by __sctp_setsockopt_connectx() and
sctp_inet_connect(), the latter has done addr_size check with size
of sa_family_t.
In the next patch to clean up __sctp_connect(), we will remove
addr_size check with size of sa_family_t from __sctp_connect()
for the 1st address.
So before doing that, __sctp_setsockopt_connectx() should do
this check first, as sctp_inet_connect() does.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index aa80cda..5f92e4a 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1311,7 +1311,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
pr_debug("%s: sk:%p addrs:%p addrs_size:%d\n",
__func__, sk, addrs, addrs_size);
- if (unlikely(addrs_size <= 0))
+ if (unlikely(addrs_size < sizeof(sa_family_t)))
return -EINVAL;
kaddrs = memdup_user(addrs, addrs_size);
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/4] sctp: clean up __sctp_connect function
From: Xin Long @ 2019-07-22 17:37 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patchset is to factor out some common code for
sctp_sendmsg_new_asoc() and __sctp_connect() into 2
new functioins.
Xin Long (4):
sctp: check addr_size with sa_family_t size in
__sctp_setsockopt_connectx
sctp: clean up __sctp_connect
sctp: factor out sctp_connect_new_asoc
sctp: factor out sctp_connect_add_peer
net/sctp/socket.c | 377 +++++++++++++++++++++---------------------------------
1 file changed, 147 insertions(+), 230 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH] net: sfc: falcon: convert to i2c_new_dummy_device
From: Wolfram Sang @ 2019-07-22 17:26 UTC (permalink / raw)
To: linux-i2c
Cc: Wolfram Sang, Solarflare linux maintainers, Edward Cree,
Martin Habets, David S. Miller, netdev, linux-kernel
Move from i2c_new_dummy() to i2c_new_dummy_device(). So, we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Only build tested. Part of a tree-wide move to deprecate
i2c_new_dummy().
drivers/net/ethernet/sfc/falcon/falcon_boards.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/sfc/falcon/falcon_boards.c b/drivers/net/ethernet/sfc/falcon/falcon_boards.c
index 839189dab98e..189ce9b9dfa7 100644
--- a/drivers/net/ethernet/sfc/falcon/falcon_boards.c
+++ b/drivers/net/ethernet/sfc/falcon/falcon_boards.c
@@ -454,13 +454,13 @@ static int sfe4001_init(struct ef4_nic *efx)
#if IS_ENABLED(CONFIG_SENSORS_LM90)
board->hwmon_client =
- i2c_new_device(&board->i2c_adap, &sfe4001_hwmon_info);
+ i2c_new_client_device(&board->i2c_adap, &sfe4001_hwmon_info);
#else
board->hwmon_client =
- i2c_new_dummy(&board->i2c_adap, sfe4001_hwmon_info.addr);
+ i2c_new_dummy_device(&board->i2c_adap, sfe4001_hwmon_info.addr);
#endif
- if (!board->hwmon_client)
- return -EIO;
+ if (IS_ERR(board->hwmon_client))
+ return PTR_ERR(board->hwmon_client);
/* Raise board/PHY high limit from 85 to 90 degrees Celsius */
rc = i2c_smbus_write_byte_data(board->hwmon_client,
@@ -468,9 +468,9 @@ static int sfe4001_init(struct ef4_nic *efx)
if (rc)
goto fail_hwmon;
- board->ioexp_client = i2c_new_dummy(&board->i2c_adap, PCA9539);
- if (!board->ioexp_client) {
- rc = -EIO;
+ board->ioexp_client = i2c_new_dummy_device(&board->i2c_adap, PCA9539);
+ if (IS_ERR(board->ioexp_client)) {
+ rc = PTR_ERR(board->ioexp_client);
goto fail_hwmon;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 6/7] dt-bindings: net: realtek: Add property to configure LED mode
From: Matthias Kaehlcke @ 2019-07-22 17:14 UTC (permalink / raw)
To: Rob Herring
Cc: Florian Fainelli, David S . Miller, Mark Rutland, Andrew Lunn,
Heiner Kallweit, netdev, devicetree, linux-kernel@vger.kernel.org,
Douglas Anderson
In-Reply-To: <CAL_JsqL_AU+JV0c2mNbXiPh2pvfYbPbLV-2PHHX0hC3vUH4QWg@mail.gmail.com>
On Wed, Jul 10, 2019 at 09:55:12AM -0600, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 5:23 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Hi Florian,
> >
> > On Wed, Jul 03, 2019 at 02:37:47PM -0700, Florian Fainelli wrote:
> > > On 7/3/19 12:37 PM, Matthias Kaehlcke wrote:
> > > > The LED behavior of some Realtek PHYs is configurable. Add the
> > > > property 'realtek,led-modes' to specify the configuration of the
> > > > LEDs.
> > > >
> > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > > ---
> > > > Changes in v2:
> > > > - patch added to the series
> > > > ---
> > > > .../devicetree/bindings/net/realtek.txt | 9 +++++++++
> > > > include/dt-bindings/net/realtek.h | 17 +++++++++++++++++
> > > > 2 files changed, 26 insertions(+)
> > > > create mode 100644 include/dt-bindings/net/realtek.h
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/realtek.txt b/Documentation/devicetree/bindings/net/realtek.txt
> > > > index 71d386c78269..40b0d6f9ee21 100644
> > > > --- a/Documentation/devicetree/bindings/net/realtek.txt
> > > > +++ b/Documentation/devicetree/bindings/net/realtek.txt
> > > > @@ -9,6 +9,12 @@ Optional properties:
> > > >
> > > > SSC is only available on some Realtek PHYs (e.g. RTL8211E).
> > > >
> > > > +- realtek,led-modes: LED mode configuration.
> > > > +
> > > > + A 0..3 element vector, with each element configuring the operating
> > > > + mode of an LED. Omitted LEDs are turned off. Allowed values are
> > > > + defined in "include/dt-bindings/net/realtek.h".
> > >
> > > This should probably be made more general and we should define LED modes
> > > that makes sense regardless of the PHY device, introduce a set of
> > > generic functions for validating and then add new function pointer for
> > > setting the LED configuration to the PHY driver. This would allow to be
> > > more future proof where each PHY driver could expose standard LEDs class
> > > devices to user-space, and it would also allow facilities like: ethtool
> > > -p to plug into that.
> > >
> > > Right now, each driver invents its own way of configuring LEDs, that
> > > does not scale, and there is not really a good reason for that other
> > > than reviewing drivers in isolation and therefore making it harder to
> > > extract the commonality. Yes, I realize that since you are the latest
> > > person submitting something in that area, you are being selected :)
>
> I agree.
>
> > I see the merit of your proposal to come up with a generic mechanism
> > to configure Ethernet LEDs, however I can't justify spending much of
> > my work time on this. If it is deemed useful I'm happy to send another
> > version of the current patchset that addresses the reviewer's comments,
> > but if the implementation of a generic LED configuration interface is
> > a requirement I will have to abandon at least the LED configuration
> > part of this series.
>
> Can you at least define a common binding for this. Maybe that's just
> removing 'realtek'. While the kernel side can evolve to a common
> infrastructure, the DT bindings can't.
I'm working on a generic binding.
I wonder what is the best process for reviewing/landing it, I'm
doubting between two options:
a) only post the binding doc and the generic PHY code that reads
the configuration from the DT. Post Realtek patches once
the binding/generic code has been acked.
pros: no churn from Realtek specific patches
cons: initially no (real) user of the new binding
b) post generic and Realtek changes together
pros: the binding has a user initially
cons: churn from Realtek specific patches
I can do either, depending on what maintainers/reviewers prefer. I'm
slightly inclined towards a)
^ permalink raw reply
* b53 DSA : vlan tagging broken ?
From: Anand Raj Manickam @ 2019-07-22 16:57 UTC (permalink / raw)
To: f.fainelli, netdev, andrew
Hi ,
I had working DSA with 4.9.184 kernel, with BCM53125, rev 4 hardware .
It had 2 bridges with
br0 8000.00 no lan1
lan2
lan3
eth0.101
br1 8000.01 no eth0.102
wan
# bridge vlan
port vlan ids
wan 102 PVID Egress Untagged
wan 102 PVID Egress Untagged
lan3 101 PVID Egress Untagged
lan3 101 PVID Egress Untagged
lan2 101 PVID Egress Untagged
lan2 101 PVID Egress Untagged
lan1 101 PVID Egress Untagged
lan1 101 PVID Egress Untagged
eth0.102 102 PVID
eth0.102
br1 1 PVID Egress Untagged
eth0.101 101 PVID
eth0.101
br0 1 PVID Egress Untagged
I upgrade the kernel to 5.2 . The behavior is broken. I had to rip the
config and check what was broken from the init scripts.
the bridge vlan commands failed to add , as the newer kernel requires
the vlan interfaces to be up .
https://lkml.org/lkml/2018/5/22/887 - i had the same behaviour as this thread .
I re added them manually , so the we have the same bridge to vlan
mapping as the previous kernel .
but the ingress packets for WAN where going to LAN(bridge) and the
egress packets where on WAN(bridge) but the packets never leaves the
interface .
I test this with a simple config :
ip link add link eth0 name eth0.101 type vlan id 101
ip link add link eth0 name eth0.102 type vlan id 102
ip link set eth0.101 up
ip link set eth0.102 up
ip link add br0 type bridge
ip link add br1 type bridge
ip link set lan1 master br1
ip link set lan2 master br1
ip link set lan3 master br1
ip link set wan master br0
bridge vlan add vid 101 dev lan1 pvid untagged
bridge vlan add vid 101 dev lan2 pvid untagged
bridge vlan add vid 101 dev lan3 pvid untagged
bridge vlan add vid 102 dev wan pvid untagged
bridge vlan del vid 1 dev wan
bridge vlan del vid 1 dev lan1
bridge vlan del vid 1 dev lan2
bridge vlan del vid 1 dev lan3
ip link set eth0.101 master br1
ip link set eth0.102 master br0
bridge vlan del vid 1 dev eth0.102
bridge vlan del vid 1 dev eth0.101
bridge vlan add vid 102 dev eth0.102 pvid
bridge vlan add vid 101 dev eth0.101 pvid
ifconfig br0 up
ifconfig br1 up
ifconfig wan up
ifconfig lan1 up
ifconfig lan2 up
ifconfig lan3 up
I donot see any packets with a tag on eth0
~# bridge vlan
port vlan ids
wan 102 PVID Egress Untagged
lan3 101 PVID Egress Untagged
lan2 101 PVID Egress Untagged
lan1 101 PVID Egress Untagged
eth0.101 101 PVID
eth0.102 102 PVID
br0 1 PVID Egress Untagged
br1 1 PVID Egress Untagged
These are the loaded modules:
# lsmod
Module Size Used by
b53_mdio 16384 0
b53_mmap 16384 0
b53_common 28672 2 b53_mdio,b53_mmap
tag_8021q 16384 0
dsa_core 32768 9 b53_mdio,b53_common,b53_mmap,tag_8021q
phylink 20480 2 b53_common,dsa_core
if i re config
#bridge vlan add vid 102 dev wan pvid untagged
#bridge vlan add vid 102 dev eth0.102 pvid
Then i see the tags for ingress packets . but no packets are
transmitted out on the wire , but the stats in ifconfig show as
transmitted .
# ifconfig br0
br0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 10.17.33.137 netmask 255.255.255.0 broadcast 10.17.33.255
inet6 fe80::3ef8:4aff:fe9c:5a04 prefixlen 64 scopeid 0x20<link>
ether 3c:f8:4a:9c:5a:04 txqueuelen 1000 (Ethernet)
RX packets 616 bytes 32351 (31.5 KiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 679 bytes 30286 (29.5 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
#ifconfig eth0
eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet6 fe80::d6:5ff:fec2:93af prefixlen 64 scopeid 0x20<link>
ether 02:d6:05:c2:93:af txqueuelen 1000 (Ethernet)
RX packets 58017 bytes 4004093 (3.8 MiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 4322 bytes 301365 (294.3 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
device interrupt 56
Can some shed some light on this config .
-Anand
^ 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