* [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
The doubling of the indirect calls caused a too big regression for some
benchmarks in our post-spectre world.
With some of the nastiness in the networking code moved out of the way
we can instead stick a pointer to a waitqueue into struct file and
avoid one of the indirect calls. This actually happens to simplify
the code quite a bit as well.
Note that for this removes the possibility of actually returning an
error before waiting in poll. We could still do this with an ERR_PTR
in f_poll_head with a little bit of WRITE_ONCE/READ_ONCE magic, but
I'd like to defer that until actually required.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/filesystems/Locking | 4 +---
Documentation/filesystems/vfs.txt | 15 +++++---------
drivers/char/random.c | 8 ++++----
fs/aio.c | 22 ++++++---------------
fs/eventfd.c | 33 +++++++++++++++++++------------
fs/eventpoll.c | 9 +--------
fs/pipe.c | 12 +++--------
fs/select.c | 18 ++++-------------
fs/timerfd.c | 31 +++++++++++++++++------------
include/linux/fs.h | 2 +-
include/linux/poll.h | 7 +------
net/socket.c | 17 +++-------------
12 files changed, 67 insertions(+), 111 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 2c391338c675..4d183e8258b6 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -441,7 +441,6 @@ prototypes:
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
@@ -505,8 +504,7 @@ in sys_read() and friends.
the lease within the individual filesystem to record the result of the
operation
-->poll_mask can be called with or without the waitqueue lock for the waitqueue
-returned from ->get_poll_head.
+->poll_mask can be called with or without the waitqueue lock
--------------------------- dquot_operations -------------------------------
prototypes:
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 829a7b7857a4..2d2f07acafa8 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -857,7 +857,6 @@ struct file_operations {
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
@@ -903,16 +902,12 @@ otherwise noted.
activity on this file and (optionally) go to sleep until there
is activity. Called by the select(2) and poll(2) system calls
- get_poll_head: Returns the struct wait_queue_head that callers can
- wait on. Callers need to check the returned events using ->poll_mask
- once woken. Can return NULL to indicate polling is not supported,
- or any error code using the ERR_PTR convention to indicate that a
- grave error occured and ->poll_mask shall not be called.
-
poll_mask: return the mask of EPOLL* values describing the file descriptor
- state. Called either before going to sleep on the waitqueue returned by
- get_poll_head, or after it has been woken. If ->get_poll_head and
- ->poll_mask are implemented ->poll does not need to be implement.
+ state. Called either before going to sleep on ->f_poll_head, or after it
+ has been woken. On files that support ->poll_mask the ->f_poll_head field
+ must point to a wait_queue_head that poll can wait on. The waitqueue must
+ have the same (or a longer) life time as the struct file itself.
+ If ->poll_mask is implemented ->poll does not need to be implement.
unlocked_ioctl: called by the ioctl(2) system call.
diff --git a/drivers/char/random.c b/drivers/char/random.c
index a8fb0020ba5c..e6260a9fa3b9 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1875,10 +1875,10 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
return ret;
}
-static struct wait_queue_head *
-random_get_poll_head(struct file *file, __poll_t events)
+static int random_open(struct inode *inode, struct file *file)
{
- return &random_wait;
+ file->f_poll_head = &random_wait;
+ return 0;
}
static __poll_t
@@ -1990,9 +1990,9 @@ static int random_fasync(int fd, struct file *filp, int on)
}
const struct file_operations random_fops = {
+ .open = random_open,
.read = random_read,
.write = random_write,
- .get_poll_head = random_get_poll_head,
.poll_mask = random_poll_mask,
.unlocked_ioctl = random_ioctl,
.fasync = random_fasync,
diff --git a/fs/aio.c b/fs/aio.c
index e1d20124ec0e..84f498df8afc 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -168,7 +168,6 @@ struct fsync_iocb {
struct poll_iocb {
struct file *file;
__poll_t events;
- struct wait_queue_head *head;
union {
struct wait_queue_entry wait;
@@ -1632,7 +1631,7 @@ static int aio_poll_cancel(struct kiocb *iocb)
{
struct aio_kiocb *aiocb = container_of(iocb, struct aio_kiocb, rw);
struct poll_iocb *req = &aiocb->poll;
- struct wait_queue_head *head = req->head;
+ struct wait_queue_head *head = req->file->f_poll_head;
bool found = false;
spin_lock(&head->lock);
@@ -1655,7 +1654,7 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
struct file *file = req->file;
__poll_t mask = key_to_poll(key);
- assert_spin_locked(&req->head->lock);
+ assert_spin_locked(&file->f_poll_head->lock);
/* for instances that support it check for an event match first: */
if (mask && !(mask & req->events))
@@ -1703,30 +1702,21 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb)
req->file = fget(iocb->aio_fildes);
if (unlikely(!req->file))
return -EBADF;
- if (!file_has_poll_mask(req->file))
+ if (!req->file->f_poll_head)
goto out_fail;
- req->head = req->file->f_op->get_poll_head(req->file, req->events);
- if (!req->head)
- goto out_fail;
- if (IS_ERR(req->head)) {
- mask = EPOLLERR;
- goto done;
- }
-
init_waitqueue_func_entry(&req->wait, aio_poll_wake);
aiocb->ki_cancel = aio_poll_cancel;
spin_lock_irq(&ctx->ctx_lock);
- spin_lock(&req->head->lock);
+ spin_lock(&req->file->f_poll_head->lock);
mask = req->file->f_op->poll_mask(req->file, req->events) & req->events;
if (!mask) {
- __add_wait_queue(req->head, &req->wait);
+ __add_wait_queue(req->file->f_poll_head, &req->wait);
list_add_tail(&aiocb->ki_list, &ctx->active_reqs);
}
- spin_unlock(&req->head->lock);
+ spin_unlock(&req->file->f_poll_head->lock);
spin_unlock_irq(&ctx->ctx_lock);
-done:
if (mask)
__aio_poll_complete(aiocb, mask);
return 0;
diff --git a/fs/eventfd.c b/fs/eventfd.c
index ceb1031f1cac..8904b127577b 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -101,14 +101,6 @@ static int eventfd_release(struct inode *inode, struct file *file)
return 0;
}
-static struct wait_queue_head *
-eventfd_get_poll_head(struct file *file, __poll_t events)
-{
- struct eventfd_ctx *ctx = file->private_data;
-
- return &ctx->wqh;
-}
-
static __poll_t eventfd_poll_mask(struct file *file, __poll_t eventmask)
{
struct eventfd_ctx *ctx = file->private_data;
@@ -311,7 +303,6 @@ static const struct file_operations eventfd_fops = {
.show_fdinfo = eventfd_show_fdinfo,
#endif
.release = eventfd_release,
- .get_poll_head = eventfd_get_poll_head,
.poll_mask = eventfd_poll_mask,
.read = eventfd_read,
.write = eventfd_write,
@@ -390,7 +381,8 @@ EXPORT_SYMBOL_GPL(eventfd_ctx_fileget);
static int do_eventfd(unsigned int count, int flags)
{
struct eventfd_ctx *ctx;
- int fd;
+ struct file *file;
+ int fd, error;
/* Check the EFD_* constants for consistency. */
BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
@@ -408,12 +400,27 @@ static int do_eventfd(unsigned int count, int flags)
ctx->count = count;
ctx->flags = flags;
- fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
+ error = get_unused_fd_flags(O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
+ if (error < 0)
+ goto err_free_ctx;
+ fd = error;
+
+ file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
- if (fd < 0)
- eventfd_free_ctx(ctx);
+ if (IS_ERR(file)) {
+ error = PTR_ERR(file);
+ goto err_put_unused_fd;
+ }
+ file->f_poll_head = &ctx->wqh;
+ fd_install(fd, file);
return fd;
+
+err_put_unused_fd:
+ put_unused_fd(fd);
+err_free_ctx:
+ eventfd_free_ctx(ctx);
+ return error;
}
SYSCALL_DEFINE2(eventfd2, unsigned int, count, int, flags)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index ea4436f409fb..a0d199be91db 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -922,13 +922,6 @@ static __poll_t ep_read_events_proc(struct eventpoll *ep, struct list_head *head
return 0;
}
-static struct wait_queue_head *ep_eventpoll_get_poll_head(struct file *file,
- __poll_t eventmask)
-{
- struct eventpoll *ep = file->private_data;
- return &ep->poll_wait;
-}
-
static __poll_t ep_eventpoll_poll_mask(struct file *file, __poll_t eventmask)
{
struct eventpoll *ep = file->private_data;
@@ -972,7 +965,6 @@ static const struct file_operations eventpoll_fops = {
.show_fdinfo = ep_show_fdinfo,
#endif
.release = ep_eventpoll_release,
- .get_poll_head = ep_eventpoll_get_poll_head,
.poll_mask = ep_eventpoll_poll_mask,
.llseek = noop_llseek,
};
@@ -1973,6 +1965,7 @@ static int do_epoll_create(int flags)
goto out_free_fd;
}
ep->file = file;
+ file->f_poll_head = &ep->poll_wait;
fd_install(fd, file);
return fd;
diff --git a/fs/pipe.c b/fs/pipe.c
index bb0840e234f3..6817a60bebc9 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -509,14 +509,6 @@ static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
}
}
-static struct wait_queue_head *
-pipe_get_poll_head(struct file *filp, __poll_t events)
-{
- struct pipe_inode_info *pipe = filp->private_data;
-
- return &pipe->wait;
-}
-
/* No kernel lock held - fine */
static __poll_t pipe_poll_mask(struct file *filp, __poll_t events)
{
@@ -768,6 +760,7 @@ int create_pipe_files(struct file **res, int flags)
f->f_flags = O_WRONLY | (flags & (O_NONBLOCK | O_DIRECT));
f->private_data = inode->i_pipe;
+ f->f_poll_head = &inode->i_pipe->wait;
res[0] = alloc_file(&path, FMODE_READ, &pipefifo_fops);
if (IS_ERR(res[0])) {
@@ -778,6 +771,7 @@ int create_pipe_files(struct file **res, int flags)
path_get(&path);
res[0]->private_data = inode->i_pipe;
res[0]->f_flags = O_RDONLY | (flags & O_NONBLOCK);
+ res[0]->f_poll_head = &inode->i_pipe->wait;
res[1] = f;
return 0;
@@ -930,6 +924,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
/* We can only do regular read/write on fifos */
filp->f_mode &= (FMODE_READ | FMODE_WRITE);
+ filp->f_poll_head = &pipe->wait;
switch (filp->f_mode) {
case FMODE_READ:
@@ -1023,7 +1018,6 @@ const struct file_operations pipefifo_fops = {
.llseek = no_llseek,
.read_iter = pipe_read,
.write_iter = pipe_write,
- .get_poll_head = pipe_get_poll_head,
.poll_mask = pipe_poll_mask,
.unlocked_ioctl = pipe_ioctl,
.release = pipe_release,
diff --git a/fs/select.c b/fs/select.c
index 317891ff8165..26e20063454a 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -38,20 +38,10 @@ __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
{
if (file->f_op->poll) {
return file->f_op->poll(file, pt);
- } else if (file_has_poll_mask(file)) {
- unsigned int events = poll_requested_events(pt);
- struct wait_queue_head *head;
-
- if (pt && pt->_qproc) {
- head = file->f_op->get_poll_head(file, events);
- if (!head)
- return DEFAULT_POLLMASK;
- if (IS_ERR(head))
- return EPOLLERR;
- pt->_qproc(file, head, pt);
- }
-
- return file->f_op->poll_mask(file, events);
+ } else if (file->f_poll_head) {
+ if (pt && pt->_qproc)
+ pt->_qproc(file, file->f_poll_head, pt);
+ return file->f_op->poll_mask(file, poll_requested_events(pt));
} else {
return DEFAULT_POLLMASK;
}
diff --git a/fs/timerfd.c b/fs/timerfd.c
index d84a2bee4f82..08e820166c4d 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -227,14 +227,6 @@ static int timerfd_release(struct inode *inode, struct file *file)
return 0;
}
-static struct wait_queue_head *timerfd_get_poll_head(struct file *file,
- __poll_t eventmask)
-{
- struct timerfd_ctx *ctx = file->private_data;
-
- return &ctx->wqh;
-}
-
static __poll_t timerfd_poll_mask(struct file *file, __poll_t eventmask)
{
struct timerfd_ctx *ctx = file->private_data;
@@ -363,7 +355,6 @@ static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg
static const struct file_operations timerfd_fops = {
.release = timerfd_release,
- .get_poll_head = timerfd_get_poll_head,
.poll_mask = timerfd_poll_mask,
.read = timerfd_read,
.llseek = noop_llseek,
@@ -386,8 +377,9 @@ static int timerfd_fget(int fd, struct fd *p)
SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
{
- int ufd;
+ int ufd, error;
struct timerfd_ctx *ctx;
+ struct file *file;
/* Check the TFD_* constants for consistency. */
BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
@@ -424,12 +416,25 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
ctx->moffs = ktime_mono_to_real(0);
- ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
+ error = get_unused_fd_flags(O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
+ if (error < 0)
+ goto out_free_ctx;
+ ufd = error;
+
+ file = anon_inode_getfile("[timerfd]", &timerfd_fops, ctx,
O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
- if (ufd < 0)
- kfree(ctx);
+ if (IS_ERR(file)) {
+ error = PTR_ERR(file);
+ goto err_put_unused_fd;
+ }
+ file->f_poll_head = &ctx->wqh;
+ fd_install(ufd, file);
return ufd;
+err_put_unused_fd:
+ put_unused_fd(ufd);
+out_free_ctx:
+ return error;
}
static int do_timerfd_settime(int ufd, int flags,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5c91108846db..cc0fb83d3743 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -878,6 +878,7 @@ struct file {
loff_t f_pos;
struct fown_struct f_owner;
const struct cred *f_cred;
+ struct wait_queue_head *f_poll_head;
struct file_ra_state f_ra;
u64 f_version;
@@ -1720,7 +1721,6 @@ struct file_operations {
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
- struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
__poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index fdf86b4cbc71..e3dd9dfee20a 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,14 +74,9 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
-static inline bool file_has_poll_mask(struct file *file)
-{
- return file->f_op->get_poll_head && file->f_op->poll_mask;
-}
-
static inline bool file_can_poll(struct file *file)
{
- return file->f_op->poll || file_has_poll_mask(file);
+ return file->f_op->poll || file->f_poll_head;
}
__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt);
diff --git a/net/socket.c b/net/socket.c
index 4354afe8ad96..155d4ba38645 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -117,8 +117,6 @@ static ssize_t sock_write_iter(struct kiocb *iocb, struct iov_iter *from);
static int sock_mmap(struct file *file, struct vm_area_struct *vma);
static int sock_close(struct inode *inode, struct file *file);
-static struct wait_queue_head *sock_get_poll_head(struct file *file,
- __poll_t events);
static __poll_t sock_poll_mask(struct file *file, __poll_t);
static __poll_t sock_poll(struct file *file, struct poll_table_struct *wait);
static long sock_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
@@ -143,7 +141,6 @@ static const struct file_operations socket_file_ops = {
.llseek = no_llseek,
.read_iter = sock_read_iter,
.write_iter = sock_write_iter,
- .get_poll_head = sock_get_poll_head,
.poll_mask = sock_poll_mask,
.poll = sock_poll,
.unlocked_ioctl = sock_ioctl,
@@ -422,6 +419,8 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
sock->file = file;
file->f_flags = O_RDWR | (flags & O_NONBLOCK);
+ if (sock->ops->poll_mask)
+ file->f_poll_head = &sock->wq->wait;
file->private_data = sock;
return file;
}
@@ -1128,16 +1127,6 @@ int sock_create_lite(int family, int type, int protocol, struct socket **res)
}
EXPORT_SYMBOL(sock_create_lite);
-static struct wait_queue_head *sock_get_poll_head(struct file *file,
- __poll_t events)
-{
- struct socket *sock = file->private_data;
-
- if (!sock->ops->poll_mask)
- return NULL;
- return &sock->wq->wait;
-}
-
static __poll_t sock_poll_mask(struct file *file, __poll_t events)
{
struct socket *sock = file->private_data;
@@ -1167,7 +1156,7 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
if (sock->ops->poll) {
mask = sock->ops->poll(file, sock, wait);
} else if (sock->ops->poll_mask) {
- sock_poll_wait(file, sock_get_poll_head(file, events), wait);
+ sock_poll_wait(file, &sock->wq->wait, wait);
mask = sock->ops->poll_mask(sock, events);
}
--
2.17.1
^ permalink raw reply related
* [PATCH 5/6] net: remove sock_poll_busy_loop
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
There is no point in hiding this logic in a helper. Also remove the
useless events != 0 check, and reorder the remaining ones so that the
cheaper test comes first.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/net/busy_poll.h | 9 ---------
net/socket.c | 4 +++-
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 4a459a0d70d1..71c72a939bf8 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -121,15 +121,6 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock)
#endif
}
-static inline void sock_poll_busy_loop(struct socket *sock, __poll_t events)
-{
- if (sk_can_busy_loop(sock->sk) &&
- events && (events & POLL_BUSY_LOOP)) {
- /* once, only if requested by syscall */
- sk_busy_loop(sock->sk, 1);
- }
-}
-
/* used in the NIC receive handler to mark the skb */
static inline void skb_mark_napi_id(struct sk_buff *skb,
struct napi_struct *napi)
diff --git a/net/socket.c b/net/socket.c
index ca300c97b7ba..4354afe8ad96 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1160,7 +1160,9 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
struct socket *sock = file->private_data;
__poll_t events = poll_requested_events(wait), mask = 0;
- sock_poll_busy_loop(sock, events);
+ /* poll once if requested by the syscall */
+ if ((events & POLL_BUSY_LOOP) && sk_can_busy_loop(sock->sk))
+ sk_busy_loop(sock->sk, 1);
if (sock->ops->poll) {
mask = sock->ops->poll(file, sock, wait);
--
2.17.1
^ permalink raw reply related
* [PATCH 4/6] net: remove busy polling from sock_get_poll_head
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
Busy polling always comes from a synchronous poll context, so for now we
can assume that it calls ->poll if present. Move the busy polling in
sock_poll to the common block and remove it from sock_get_poll_head to
prepare for the removal of the get_poll_head method.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 7cf037d21805..ca300c97b7ba 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1135,7 +1135,6 @@ static struct wait_queue_head *sock_get_poll_head(struct file *file,
if (!sock->ops->poll_mask)
return NULL;
- sock_poll_busy_loop(sock, events);
return &sock->wq->wait;
}
@@ -1161,8 +1160,9 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
struct socket *sock = file->private_data;
__poll_t events = poll_requested_events(wait), mask = 0;
+ sock_poll_busy_loop(sock, events);
+
if (sock->ops->poll) {
- sock_poll_busy_loop(sock, events);
mask = sock->ops->poll(file, sock, wait);
} else if (sock->ops->poll_mask) {
sock_poll_wait(file, sock_get_poll_head(file, events), wait);
--
2.17.1
^ permalink raw reply related
* [PATCH 3/6] net: don't detour through struct to find the poll head
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
As far as I can tell sock->sk->sk_wq->wait will always point to
sock->wq->wait. That means we can do the shorter dereference and
not worry about any RCU protection.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/socket.c b/net/socket.c
index fe6620607b07..7cf037d21805 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1136,7 +1136,7 @@ static struct wait_queue_head *sock_get_poll_head(struct file *file,
if (!sock->ops->poll_mask)
return NULL;
sock_poll_busy_loop(sock, events);
- return sk_sleep(sock->sk);
+ return &sock->wq->wait;
}
static __poll_t sock_poll_mask(struct file *file, __poll_t events)
--
2.17.1
^ permalink raw reply related
* [PATCH 2/6] net: remove bogus RCU annotations on socket.wq
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
We never use RCU protection for it, just a lot of cargo-cult
rcu_deference_protects calls.
Note that we do keep the kfree_rcu call for it, as the references through
struct sock are RCU protected and thus might require a grace period before
freeing.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/net.h | 2 +-
include/net/sock.h | 2 +-
net/socket.c | 10 ++++------
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index 08b6eb964dd6..2be0d5368315 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -114,7 +114,7 @@ struct socket {
unsigned long flags;
- struct socket_wq __rcu *wq;
+ struct socket_wq *wq;
struct file *file;
struct sock *sk;
diff --git a/include/net/sock.h b/include/net/sock.h
index b3b75419eafe..ad85d37c83c8 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1725,7 +1725,7 @@ static inline void sock_graft(struct sock *sk, struct socket *parent)
{
WARN_ON(parent->sk);
write_lock_bh(&sk->sk_callback_lock);
- sk->sk_wq = parent->wq;
+ rcu_assign_pointer(sk->sk_wq, parent->wq);
parent->sk = sk;
sk_set_socket(sk, parent);
sk->sk_uid = SOCK_INODE(parent)->i_uid;
diff --git a/net/socket.c b/net/socket.c
index 0f397fa33614..fe6620607b07 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -255,7 +255,7 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
init_waitqueue_head(&wq->wait);
wq->fasync_list = NULL;
wq->flags = 0;
- RCU_INIT_POINTER(ei->socket.wq, wq);
+ ei->socket.wq = wq;
ei->socket.state = SS_UNCONNECTED;
ei->socket.flags = 0;
@@ -269,11 +269,9 @@ static struct inode *sock_alloc_inode(struct super_block *sb)
static void sock_destroy_inode(struct inode *inode)
{
struct socket_alloc *ei;
- struct socket_wq *wq;
ei = container_of(inode, struct socket_alloc, vfs_inode);
- wq = rcu_dereference_protected(ei->socket.wq, 1);
- kfree_rcu(wq, rcu);
+ kfree_rcu(ei->socket.wq, rcu);
kmem_cache_free(sock_inode_cachep, ei);
}
@@ -607,7 +605,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
module_put(owner);
}
- if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
+ if (sock->wq->fasync_list)
pr_err("%s: fasync list not empty!\n", __func__);
if (!sock->file) {
@@ -1211,7 +1209,7 @@ static int sock_fasync(int fd, struct file *filp, int on)
return -EINVAL;
lock_sock(sk);
- wq = rcu_dereference_protected(sock->wq, lockdep_sock_is_held(sk));
+ wq = sock->wq;
fasync_helper(fd, filp, on, &wq->fasync_list);
if (!wq->fasync_list)
--
2.17.1
^ permalink raw reply related
* [PATCH 1/6] net: remove sock_poll_busy_flag
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
In-Reply-To: <20180628142059.10017-1-hch@lst.de>
Can simplify be inlined into the only caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/net/busy_poll.h | 6 ------
net/socket.c | 5 ++++-
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index c5187438af38..4a459a0d70d1 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -130,12 +130,6 @@ static inline void sock_poll_busy_loop(struct socket *sock, __poll_t events)
}
}
-/* if this socket can poll_ll, tell the system call */
-static inline __poll_t sock_poll_busy_flag(struct socket *sock)
-{
- return sk_can_busy_loop(sock->sk) ? POLL_BUSY_LOOP : 0;
-}
-
/* used in the NIC receive handler to mark the skb */
static inline void skb_mark_napi_id(struct sk_buff *skb,
struct napi_struct *napi)
diff --git a/net/socket.c b/net/socket.c
index 8a109012608a..0f397fa33614 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1171,7 +1171,10 @@ static __poll_t sock_poll(struct file *file, poll_table *wait)
mask = sock->ops->poll_mask(sock, events);
}
- return mask | sock_poll_busy_flag(sock);
+ /* this socket can poll_ll so tell the system call */
+ if (sk_can_busy_loop(sock->sk))
+ mask |= POLL_BUSY_LOOP;
+ return mask;
}
static int sock_mmap(struct file *file, struct vm_area_struct *vma)
--
2.17.1
^ permalink raw reply related
* [RFC] replace ->get_poll_head with a waitqueue pointer in struct file
From: Christoph Hellwig @ 2018-06-28 14:20 UTC (permalink / raw)
To: Alexander Viro; +Cc: Linus Torvalds, linux-fsdevel, netdev, lkp
Introducing the new poll methods showed up a regression in the
will-it-scale ltp tests. One reason for that is that indirect function
calls are very expensive now with the spectre mitigations. I'm waiting
for better numbers, but this series has shown a 5% improvements in the
ops per second so far, while for the get_poll_head addition we had
regressions of 3.7% or 8.8% depending on the measurement.
This series removes the get_poll_head method again and instead stores an
optional wait_queue_head pointer in struct file, on which the poll_mask
method can be used if it is set. The only complication is the networking
poll code which not only does some interesting gymnastics to get at the
wait queue pointer, but also has a mode to to hardware polling before
waiting for an even from poll or epoll. Because of that this series has
a few net prep patches that need careful review.
^ permalink raw reply
* Re: [RFC PATCH 00/11] OVS eBPF datapath.
From: William Tu @ 2018-06-28 14:19 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: <dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
Tom Herbert via iovisor-dev, Linux Kernel Network Developers
In-Reply-To: <20180628030023.pkgdu4brun3jy2bz-+o4/htvd0TCa6kscz5V53/3mLCh9rsb+VpNB7YpNyf8@public.gmane.org>
Hi Alexei,
Thanks a lot for the feedback!
On Wed, Jun 27, 2018 at 8:00 PM, Alexei Starovoitov
<alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Jun 23, 2018 at 05:16:32AM -0700, William Tu wrote:
>>
>> Discussion
>> ==========
>> We are still actively working on finishing the feature, currently
>> the basic forwarding and tunnel feature work, but still under
>> heavy debugging and development. The purpose of this RFC is to
>> get some early feedbacks and direction for finishing the complete
>> features in existing kernel's OVS datapath (the net/openvswitch/*).
>
> Thank you for sharing the patches.
>
>> Three major issues we are worried:
>> a. Megaflow support in BPF.
>> b. Connection Tracking support in BPF.
>
> my opinion on the above two didn't change.
> To recap:
> A. Non scalable megaflow map is no go. I'd like to see packet classification
> algorithm like hicuts or efficuts to be implemented instead, since it can be
> shared by generic bpf, bpftiler, ovs and likely others.
We did try the decision tree approach using dpdk's acl lib. The lookup
speed is 6 times faster than the magaflow using tuple space.
However, the update/insertion requires rebuilding/re-balancing the decision
tree so it's way too slow. I think hicuts or efficuts suffers the same issue.
So decision tree algos are scalable only for lookup operation due to its
optimization over tree depth, but not scalable under
update/insert/delete operations.
On customer's system we see megaflow update/insert rate around 10 rules/sec,
this makes decision tree unusable, unless we invent something to optimize the
update/insert time or incremental update of these decision tree algo.
Now my backup plan is to implement megaflow in BPF.
> B. instead of helpers to interface with conntrack the way ovs did, I prefer
> a generic conntrack mechanism that can be used out of xdp too
>
OK. We will work on this direction.
>> c. Verifier limitation.
>
> Not sure what limitations you're concerned about.
>
Mostly related to stack. The flow key OVS uses (struct sw_flow_key)
is 464 byte. We trim a lot, now around 300 byte, but still huge, considering
the BPF's stack limit is 512 byte.
We can always break the large program then tail call, but sometimes
the register spills on the stack, and when restore, the states is gone
and verifier fails. This is more difficult for us to work around.
Below is an example:
----
at 203: r7 is a const and store on stack (r10 - 248)
at 250: r2 reads (r10 - 248) back.
at 251: fails the verifier
from 27 to 201: R0=map_value(id=0,off=0,ks=4,vs=4352,imm=0)
R7=inv(id=0,umax_value=31,var_off=(0x0; 0x1f))
R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
201: (7b) *(u64 *)(r10 -256) = r0
202: (27) r7 *= 136
203: (7b) *(u64 *)(r10 -248) = r7
204: (bf) r6 = r0
205: (0f) r6 += r7
206: (b7) r8 = 2
207: (15) if r6 == 0x0 goto pc+93
R0=map_value(id=0,off=0,ks=4,vs=4352,imm=0)
R6=map_value(id=0,off=0,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R7=inv(id=0,umax_value=4216,var_off=(0x0; 0x1ff8)) R8=inv2
R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1 fp-256=map_value
208: (b7) r1 = 681061
209: (63) *(u32 *)(r10 -200) = r1
210: (18) r1 = 0x6b73616d20746573
212: (7b) *(u64 *)(r10 -208) = r1
213: (bf) r1 = r10
214: (07) r1 += -208
215: (b7) r2 = 12
216: (85) call bpf_trace_printk#6
217: (bf) r7 = r6
218: (07) r7 += 8
219: (61) r1 = *(u32 *)(r6 +8)
R0=inv(id=0) R6=map_value(id=0,off=0,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R7_w=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value
220: (15) if r1 == 0x7 goto pc+82
R0=inv(id=0) R1=inv(id=0,umax_value=4294967295,var_off=(0x0;
0xffffffff)) R6=map_value(id=0,off=0,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value
221: (55) if r1 != 0x4 goto pc+228
R0=inv(id=0) R1=inv4
R6=map_value(id=0,off=0,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value
222: (61) r1 = *(u32 *)(r9 +80)
223: (7b) *(u64 *)(r10 -264) = r1
224: (61) r6 = *(u32 *)(r9 +76)
225: (b7) r1 = 0
226: (73) *(u8 *)(r10 -198) = r1
227: (b7) r1 = 2674
228: (6b) *(u16 *)(r10 -200) = r1
229: (18) r1 = 0x6568746520746573
231: (7b) *(u64 *)(r10 -208) = r1
232: (bf) r1 = r10
233: (07) r1 += -208
234: (b7) r2 = 11
235: (85) call bpf_trace_printk#6
236: (bf) r1 = r6
237: (07) r1 += 14
238: (79) r2 = *(u64 *)(r10 -264)
239: (2d) if r1 > r2 goto pc+61
R0=inv(id=0) R1=pkt(id=0,off=14,r=14,imm=0)
R2=pkt_end(id=0,off=0,imm=0) R6=pkt(id=0,off=0,r=14,imm=0)
R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value fp-264=pkt_end
240: (71) r1 = *(u8 *)(r7 +10)
R0=inv(id=0) R1_w=pkt(id=0,off=14,r=14,imm=0)
R2=pkt_end(id=0,off=0,imm=0) R6=pkt(id=0,off=0,r=14,imm=0)
R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value fp-264=pkt_end
241: (73) *(u8 *)(r6 +0) = r1
242: (71) r1 = *(u8 *)(r7 +11)
R0=inv(id=0) R1_w=inv(id=0,umax_value=255,var_off=(0x0; 0xff))
R2=pkt_end(id=0,off=0,imm=0) R6=pkt(id=0,off=0,r=14,imm=0)
R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value fp-264=pkt_end
243: (73) *(u8 *)(r6 +1) = r1
244: (71) r1 = *(u8 *)(r7 +12)
R0=inv(id=0) R1_w=inv(id=0,umax_value=255,var_off=(0x0; 0xff))
R2=pkt_end(id=0,off=0,imm=0) R6=pkt(id=0,off=0,r=14,imm=0)
R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value fp-264=pkt_end
245: (73) *(u8 *)(r6 +2) = r1
246: (71) r1 = *(u8 *)(r7 +13)
R0=inv(id=0) R1_w=inv(id=0,umax_value=255,var_off=(0x0; 0xff))
R2=pkt_end(id=0,off=0,imm=0) R6=pkt(id=0,off=0,r=14,imm=0)
R7=map_value(id=0,off=8,ks=4,vs=4352,umax_value=4216,var_off=(0x0;
0x1ff8)) R8=inv2 R9=ctx(id=0,off=0,imm=0) R10=fp0,call_-1
fp-256=map_value fp-264=pkt_end
247: (73) *(u8 *)(r6 +3) = r1
248: (79) r4 = *(u64 *)(r10 -256)
249: (bf) r1 = r4
250: (79) r2 = *(u64 *)(r10 -248)
251: (0f) r1 += r2
math between map_value pointer and register with unbounded min value
is not allowed
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1355): https://lists.iovisor.org/g/iovisor-dev/message/1355
Mute This Topic: https://lists.iovisor.org/mt/22656941/1132507
Group Owner: iovisor-dev+owner-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [glki-iovisor-dev@m.gmane.org]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: David Ahern @ 2018-06-28 14:18 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, mlxsw, sridhar.samudrala
In-Reply-To: <20180628130907.951-1-jiri@resnulli.us>
On 6/28/18 7:08 AM, Jiri Pirko wrote:
> Create dummy device with clsact first:
> # ip link add type dummy
> # tc qdisc add dev dummy0 clsact
>
> There is no template assigned by default:
> # tc chaintemplate show dev dummy0 ingress
>
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc chaintemplate add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
> The template is now showed in the list:
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0
> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
> eth_type ipv4
>
> Add another template, this time for chain number 22:
> # tc chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
> # tc chaintemplate show dev dummy0 ingress
> chaintemplate flower chain 0
> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
> eth_type ipv4
> chaintemplate flower chain 22
> eth_type ipv4
> dst_ip 0.0.0.0/16
>
> Add a filter that fits the template:
> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop
>
> Addition of filters that does not fit the template would fail:
> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
> # tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
>
> Additions of filters to chain 22:
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
>
> Removal of a template from non-empty chain would fail:
> # tc chaintemplate del dev dummy0 ingress
> Error: The chain is not empty, unable to delete template.
> We have an error talking to the kernel, -1
Why this restriction? It's a template, so why can't it be removed
regardless of whether there are filters?
>
> Once the chain is flushed, the template could be removed:
> # tc filter del dev dummy0 ingress
> # tc chaintemplate del dev dummy0 ingress
>
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko @ 2018-06-28 14:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw
In-Reply-To: <d0d49f73-ae37-effc-7d11-1092a215c20d@mojatatu.com>
Thu, Jun 28, 2018 at 03:54:17PM CEST, jhs@mojatatu.com wrote:
>On 28/06/18 09:22 AM, Jiri Pirko wrote:
>> Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>> >
>> > On 26/06/18 03:59 AM, Jiri Pirko wrote:
>> > > From: Jiri Pirko <jiri@mellanox.com>
>> > >
>> > > For the TC clsact offload these days, some of HW drivers need
>> > > to hold a magic ball. The reason is, with the first inserted rule inside
>> > > HW they need to guess what fields will be used for the matching. If
>> > > later on this guess proves to be wrong and user adds a filter with a
>> > > different field to match, there's a problem. Mlxsw resolves it now with
>> > > couple of patterns. Those try to cover as many match fields as possible.
>> > > This aproach is far from optimal, both performance-wise and scale-wise.
>> > > Also, there is a combination of filters that in certain order won't
>> > > succeed.
>> > >
>> > >
>> > > Most of the time, when user inserts filters in chain, he knows right away
>> > > how the filters are going to look like - what type and option will they
>> > > have. For example, he knows that he will only insert filters of type
>> > > flower matching destination IP address. He can specify a template that
>> > > would cover all the filters in the chain.
>> > >
>> >
>> > Is this just restricted to hardware offload? Example it will make sense
>> > for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>> > classification). i.e it is possible that rules the user enters
>> > end up being worst case a linked list lookup, yes? And allocating
>> > space for a tuple that is not in use is a waste of space.
>>
>> I'm afraid I don't understand clearly what you say.
>
>Well - I was trying to understand what you said ;->
>
>I think what you are getting at is two issues:
>a) space in the tcams - if the user is just going to enter
>rules which use one tuple (dst ip for example) the hardware
>would be better off told that this is the case so it doesnt
>allocate space in anticipation that someone is going to
>specify src ip later on.
Yes.
>b) lookup speed in tcams - without the template hint a
>selection of rules may end up looking like a linked list
>which is not optimal for lookup
Well. Not really, but wider keys have bigger overheads in general. So
the motivation is to have the keys as small as possible for both
performance and capacity reasons.
>
>> This is not
>> restricted to hw offload. The templates apply to all filters, no matter
>> if they are offloaded or not.
>>
>
>Do you save anything with flower(in s/w) if you only added a template
>with say dst ip/mask? I can see it will make sense for u32 which is more
>flexible and protocol independent.
No benefit for flower s/w path at this point. Perhaps the hashtables
could be organized in more optimal way with the hint. I didn't look at
it.
>
>cheers,
>jamal
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jamal Hadi Salim @ 2018-06-28 13:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw
In-Reply-To: <20180628132241.GA2177@nanopsycho.orion>
On 28/06/18 09:22 AM, Jiri Pirko wrote:
> Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>>
>> On 26/06/18 03:59 AM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> For the TC clsact offload these days, some of HW drivers need
>>> to hold a magic ball. The reason is, with the first inserted rule inside
>>> HW they need to guess what fields will be used for the matching. If
>>> later on this guess proves to be wrong and user adds a filter with a
>>> different field to match, there's a problem. Mlxsw resolves it now with
>>> couple of patterns. Those try to cover as many match fields as possible.
>>> This aproach is far from optimal, both performance-wise and scale-wise.
>>> Also, there is a combination of filters that in certain order won't
>>> succeed.
>>>
>>>
>>> Most of the time, when user inserts filters in chain, he knows right away
>>> how the filters are going to look like - what type and option will they
>>> have. For example, he knows that he will only insert filters of type
>>> flower matching destination IP address. He can specify a template that
>>> would cover all the filters in the chain.
>>>
>>
>> Is this just restricted to hardware offload? Example it will make sense
>> for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>> classification). i.e it is possible that rules the user enters
>> end up being worst case a linked list lookup, yes? And allocating
>> space for a tuple that is not in use is a waste of space.
>
> I'm afraid I don't understand clearly what you say.
Well - I was trying to understand what you said ;->
I think what you are getting at is two issues:
a) space in the tcams - if the user is just going to enter
rules which use one tuple (dst ip for example) the hardware
would be better off told that this is the case so it doesnt
allocate space in anticipation that someone is going to
specify src ip later on.
b) lookup speed in tcams - without the template hint a
selection of rules may end up looking like a linked list
which is not optimal for lookup
> This is not
> restricted to hw offload. The templates apply to all filters, no matter
> if they are offloaded or not.
>
Do you save anything with flower(in s/w) if you only added a template
with say dst ip/mask? I can see it will make sense for u32 which is more
flexible and protocol independent.
cheers,
jamal
^ permalink raw reply
* [PATCH net v2] net: phy: DP83TC811: Fix diabling interrupts
From: Dan Murphy @ 2018-06-28 13:32 UTC (permalink / raw)
To: andrew, f.fainelli; +Cc: netdev, linux-kernel, Dan Murphy
Fix a bug where INT_STAT1 was written twice and
INT_STAT2 was ignored when disabling interrupts.
Fixes: b753a9faaf9a ("net: phy: DP83TC811: Introduce support for the DP83TC811 phy")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v2 - Updated commit message to add the Fixes - http://lists.openwall.net/netdev/2018/06/28/110
drivers/net/phy/dp83tc811.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index 081d99aa3985..49ac678eb2dc 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -222,7 +222,7 @@ static int dp83811_config_intr(struct phy_device *phydev)
if (err < 0)
return err;
- err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
+ err = phy_write(phydev, MII_DP83811_INT_STAT2, 0);
}
return err;
--
2.17.0.582.gccdcbd54c
^ permalink raw reply related
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko @ 2018-06-28 13:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180628130907.951-1-jiri@resnulli.us>
Oh, this is v3 already. The changelog should be:
---
v2->v3:
- patch 5:
- rebase on top of the reoffload pathset
- patch 6:
- rebase on top of the reoffload pathset
- patch 9:
- adjust to the userspace cmdline changes
v1->v2:
- patch 6:
- remove leftover extack arg in fl_hw_create_tmplt()
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko @ 2018-06-28 13:22 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw
In-Reply-To: <a0885166-cad9-f21d-32d7-3000c0083384@mojatatu.com>
Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote:
>
>On 26/06/18 03:59 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> For the TC clsact offload these days, some of HW drivers need
>> to hold a magic ball. The reason is, with the first inserted rule inside
>> HW they need to guess what fields will be used for the matching. If
>> later on this guess proves to be wrong and user adds a filter with a
>> different field to match, there's a problem. Mlxsw resolves it now with
>> couple of patterns. Those try to cover as many match fields as possible.
>> This aproach is far from optimal, both performance-wise and scale-wise.
>> Also, there is a combination of filters that in certain order won't
>> succeed.
>>
>>
>> Most of the time, when user inserts filters in chain, he knows right away
>> how the filters are going to look like - what type and option will they
>> have. For example, he knows that he will only insert filters of type
>> flower matching destination IP address. He can specify a template that
>> would cover all the filters in the chain.
>>
>
>Is this just restricted to hardware offload? Example it will make sense
>for u32 in s/ware as well (i.e flexible TCAM like TCAM based
>classification). i.e it is possible that rules the user enters
>end up being worst case a linked list lookup, yes? And allocating
>space for a tuple that is not in use is a waste of space.
I'm afraid I don't understand clearly what you say. This is not
restricted to hw offload. The templates apply to all filters, no matter
if they are offloaded or not.
>
>If yes, then I would reword the above as something like:
>
>For very flexible classifiers such as TCAM based ones,
>one could add arbitrary tuple rules which tend to be inefficient both
>from a space and lookup performance. One approach, taken by Mlxsw,
>is to assume a multi filter tuple arrangement which is inefficient
>from a space perspective when the user-specified rules dont make
>use of pre-provisioned tuple space.
>Typically users already know what tuples are of interest to them:
>for example for ipv4 route lookup purposes they may just want to
>lookup destination IP with a specified mask etc.
>This feature allows user to provide good hints to the classifier to
>optimize.
>
>
>> This patchset is providing the possibility to user to provide such
>> template to kernel and propagate it all the way down to device
>> drivers.
>>
>> See the examples below.
>>
>> Create dummy device with clsact first:
>> # ip link add type dummy
>> # tc qdisc add dev dummy0 clsact
>>
>> There is no template assigned by default:
>> # tc filter template show dev dummy0 ingress
>>
>> Add a template of type flower allowing to insert rules matching on last
>> 2 bytes of destination mac address:
>> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>
>> The template is now showed in the list:
>> # tc filter template show dev dummy0 ingress
>> filter flower chain 0
>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> eth_type ipv4
>>
>> Add another template, this time for chain number 22:
>> # tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
>> # tc filter template show dev dummy0 ingress
>> filter flower chain 0
>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
>> eth_type ipv4
>> filter flower chain 22
>> eth_type ipv4
>> dst_ip 0.0.0.0/16
>>
>> Add a filter that fits the template:
>> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop
>>
>> Addition of filters that does not fit the template would fail:
>> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> # tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>>
>> Additions of filters to chain 22:
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop
>> Error: Mask does not fit the template.
>> We have an error talking to the kernel, -1
>>
>> Removal of a template from non-empty chain would fail:
>> # tc filter template del dev dummy0 ingress
>> Error: The chain is not empty, unable to delete template.
>> We have an error talking to the kernel, -1
>>
>> Once the chain is flushed, the template could be removed:
>> # tc filter del dev dummy0 ingress
>> # tc filter template del dev dummy0 ingress
>>
>
>BTW: unlike the other comments on this - I think the syntax above
>is fine ;-> Chain are already either explicitly or are implicitly
>(case of chain 0) specified.
>
>Assuming that one cant add a new template to a chain if it already
>has at least one filter (even if no template has been added).
>
>I like it - it may help making u32 more friendly to humans in some
>cases.
>
>cheers,
>jamal
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] net: preserve sock reference when scrubbing the skb.
From: David Miller @ 2018-06-28 13:21 UTC (permalink / raw)
To: fbl; +Cc: netdev, eric.dumazet, pabeni, fw, netfilter-devel
In-Reply-To: <20180627133426.3858-1-fbl@redhat.com>
From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 27 Jun 2018 10:34:24 -0300
> The sock reference is lost when scrubbing the packet and that breaks
> TSQ (TCP Small Queues) and XPS (Transmit Packet Steering) causing
> performance impacts of about 50% in a single TCP stream when crossing
> network namespaces.
>
> XPS breaks because the queue mapping stored in the socket is not
> available, so another random queue might be selected when the stack
> needs to transmit something like a TCP ACK, or TCP Retransmissions.
> That causes packet re-ordering and/or performance issues.
>
> TSQ breaks because it orphans the packet while it is still in the
> host, so packets are queued contributing to the buffer bloat problem.
>
> Preserving the sock reference fixes both issues. The socket is
> orphaned anyways in the receiving path before any relevant action,
> but the transmit side needs some extra checking included in the
> first patch.
>
> The first patch will update netfilter to check if the socket
> netns is local before use it.
>
> The second patch removes the skb_orphan() from the skb_scrub_packet()
> and improve the documentation.
>
> ChangeLog:
> - split into two (Eric)
> - addressed Paolo's offline feedback to swap the checks in xt_socket.c
> to preserve original behavior.
> - improved ip-sysctl.txt (reported by Cong)
Series applied, thanks Flavio.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] net: preserve sock reference when scrubbing the skb.
From: David Miller @ 2018-06-28 13:20 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: fbl, netdev, eric.dumazet, pabeni, fw, netfilter-devel
In-Reply-To: <CAM_iQpXnfc8uA10EK2X7B=vB_uWayeGw=M5F-_uygu7aPvCjRw@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 27 Jun 2018 12:39:01 -0700
> Let me rephrase why I don't like this patchset:
Cong, I don't think you are seeing the situation clearly and
I am certainly going to apply this patch series even in the
face of your objections.
Suggesting that solving the lack of back pressure on a UDP
socket caused by this problem by using cgroups or cpu
usage controllers is just complete and utter madness.
^ permalink raw reply
* Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jamal Hadi Salim @ 2018-06-28 13:13 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, xiyou.wangcong, jakub.kicinski, simon.horman, john.hurley,
dsahern, mlxsw
In-Reply-To: <20180626080000.12964-1-jiri@resnulli.us>
On 26/06/18 03:59 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> For the TC clsact offload these days, some of HW drivers need
> to hold a magic ball. The reason is, with the first inserted rule inside
> HW they need to guess what fields will be used for the matching. If
> later on this guess proves to be wrong and user adds a filter with a
> different field to match, there's a problem. Mlxsw resolves it now with
> couple of patterns. Those try to cover as many match fields as possible.
> This aproach is far from optimal, both performance-wise and scale-wise.
> Also, there is a combination of filters that in certain order won't
> succeed.
>
>
> Most of the time, when user inserts filters in chain, he knows right away
> how the filters are going to look like - what type and option will they
> have. For example, he knows that he will only insert filters of type
> flower matching destination IP address. He can specify a template that
> would cover all the filters in the chain.
>
Is this just restricted to hardware offload? Example it will make sense
for u32 in s/ware as well (i.e flexible TCAM like TCAM based
classification). i.e it is possible that rules the user enters
end up being worst case a linked list lookup, yes? And allocating
space for a tuple that is not in use is a waste of space.
If yes, then I would reword the above as something like:
For very flexible classifiers such as TCAM based ones,
one could add arbitrary tuple rules which tend to be inefficient both
from a space and lookup performance. One approach, taken by Mlxsw,
is to assume a multi filter tuple arrangement which is inefficient
from a space perspective when the user-specified rules dont make
use of pre-provisioned tuple space.
Typically users already know what tuples are of interest to them:
for example for ipv4 route lookup purposes they may just want to
lookup destination IP with a specified mask etc.
This feature allows user to provide good hints to the classifier to
optimize.
> This patchset is providing the possibility to user to provide such
> template to kernel and propagate it all the way down to device
> drivers.
>
> See the examples below.
>
> Create dummy device with clsact first:
> # ip link add type dummy
> # tc qdisc add dev dummy0 clsact
>
> There is no template assigned by default:
> # tc filter template show dev dummy0 ingress
>
> Add a template of type flower allowing to insert rules matching on last
> 2 bytes of destination mac address:
> # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>
> The template is now showed in the list:
> # tc filter template show dev dummy0 ingress
> filter flower chain 0
> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
> eth_type ipv4
>
> Add another template, this time for chain number 22:
> # tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16
> # tc filter template show dev dummy0 ingress
> filter flower chain 0
> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff
> eth_type ipv4
> filter flower chain 22
> eth_type ipv4
> dst_ip 0.0.0.0/16
>
> Add a filter that fits the template:
> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop
>
> Addition of filters that does not fit the template would fail:
> # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
> # tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
>
> Additions of filters to chain 22:
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
> # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop
> Error: Mask does not fit the template.
> We have an error talking to the kernel, -1
>
> Removal of a template from non-empty chain would fail:
> # tc filter template del dev dummy0 ingress
> Error: The chain is not empty, unable to delete template.
> We have an error talking to the kernel, -1
>
> Once the chain is flushed, the template could be removed:
> # tc filter del dev dummy0 ingress
> # tc filter template del dev dummy0 ingress
>
BTW: unlike the other comments on this - I think the syntax above
is fine ;-> Chain are already either explicitly or are implicitly
(case of chain 0) specified.
Assuming that one cant add a new template to a chain if it already
has at least one filter (even if no template has been added).
I like it - it may help making u32 more friendly to humans in some
cases.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2 net-next 0/6] net sched actions: code style cleanup and fixes
From: David Miller @ 2018-06-28 13:12 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Wed, 27 Jun 2018 13:33:29 -0400
> The patchset fixes a few code stylistic issues and typos, as well as one
> detected by sparse semantic checker tool.
>
> No functional changes introduced.
>
> Patch 1 & 2 fix coding style bits caught by the checkpatch.pl script
> Patch 3 fixes an issue with a shadowed variable
> Patch 4 adds sizeof() operator instead of magic number for buffer length
> Patch 5 fixes typos in diagnostics messages
> Patch 6 explicitly sets unsigned char for bitwise operation
>
> v2:
> - submit for net-next
> - added Reviewed-by tags
> - use u8* instead of char* as per Davide Caratti suggestion
Series applied.
^ permalink raw reply
* [patch iproute2/net-next v2] tc: introduce support for chain templates
From: Jiri Pirko @ 2018-06-28 13:10 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180628130907.951-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v1->v2:
- moved the template handling
from "tc filter template" to "tc chaintemplate"
---
include/uapi/linux/rtnetlink.h | 7 +++
man/man8/tc.8 | 26 ++++++++++
tc/tc.c | 5 +-
tc/tc_common.h | 1 +
tc/tc_filter.c | 106 ++++++++++++++++++++++++++++++-----------
tc/tc_monitor.c | 5 +-
6 files changed, 121 insertions(+), 29 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index c3a7d8ecc7b9..dddb05e5cca8 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -150,6 +150,13 @@ enum {
RTM_NEWCACHEREPORT = 96,
#define RTM_NEWCACHEREPORT RTM_NEWCACHEREPORT
+ RTM_NEWCHAINTMPLT = 100,
+#define RTM_NEWCHAINTMPLT RTM_NEWCHAINTMPLT
+ RTM_DELCHAINTMPLT,
+#define RTM_DELCHAINTMPLT RTM_DELCHAINTMPLT
+ RTM_GETCHAINTMPLT,
+#define RTM_GETCHAINTMPLT RTM_GETCHAINTMPLT
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 840880fbdba6..3eee1aceaae4 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -58,6 +58,22 @@ tc \- show / manipulate traffic control settings
.B flowid
\fIflow-id\fR
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chaintemplate [ add | delete | get ] dev
+\fIDEV\fR
+.B [ parent
+\fIqdisc-id\fR
+.B | root ]\fR filtertype
+[ filtertype specific parameters ]
+
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chaintemplate [ add | delete | get ] block
+\fIBLOCK_INDEX\fR filtertype
+[ filtertype specific parameters ]
+
+
.B tc
.RI "[ " OPTIONS " ]"
.RI "[ " FORMAT " ]"
@@ -80,6 +96,16 @@ tc \- show / manipulate traffic control settings
.RI "[ " OPTIONS " ]"
.B filter show block
\fIBLOCK_INDEX\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chaintemplate show dev
+\fIDEV\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chaintemplate show block
+\fIBLOCK_INDEX\fR
.P
.B tc
diff --git a/tc/tc.c b/tc/tc.c
index 0d223281ba25..8a0592c45800 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -197,7 +197,8 @@ static void usage(void)
fprintf(stderr,
"Usage: tc [ OPTIONS ] OBJECT { COMMAND | help }\n"
" tc [-force] -batch filename\n"
- "where OBJECT := { qdisc | class | filter | action | monitor | exec }\n"
+ "where OBJECT := { qdisc | class | filter | chaintemplate |\n"
+ " action | monitor | exec }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[aw] |\n"
" -o[neline] | -j[son] | -p[retty] | -c[olor]\n"
" -b[atch] [filename] | -n[etns] name |\n"
@@ -212,6 +213,8 @@ static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
return do_class(argc-1, argv+1);
if (matches(*argv, "filter") == 0)
return do_filter(argc-1, argv+1, buf, buflen);
+ if (matches(*argv, "chaintemplate") == 0)
+ return do_chaintmplt(argc-1, argv+1, buf, buflen);
if (matches(*argv, "actions") == 0)
return do_action(argc-1, argv+1, buf, buflen);
if (matches(*argv, "monitor") == 0)
diff --git a/tc/tc_common.h b/tc/tc_common.h
index 49c24616c2c3..16cefe896109 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -8,6 +8,7 @@ extern struct rtnl_handle rth;
extern int do_qdisc(int argc, char **argv);
extern int do_class(int argc, char **argv);
extern int do_filter(int argc, char **argv, void *buf, size_t buflen);
+extern int do_chaintmplt(int argc, char **argv, void *buf, size_t buflen);
extern int do_action(int argc, char **argv, void *buf, size_t buflen);
extern int do_tcmonitor(int argc, char **argv);
extern int do_exec(int argc, char **argv);
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c5bb0bffe19b..df0ce853fbcc 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -39,12 +39,21 @@ static void usage(void)
"\n"
" tc filter show [ dev STRING ] [ root | ingress | egress | parent CLASSID ]\n"
" tc filter show [ block BLOCK_INDEX ]\n"
+ " tc chaintemplate [ add | del | get | show ] [ dev STRING ]\n"
+ " tc chaintemplate [ add | del | get | show ] [ block BLOCK_INDEX ] ]\n"
"Where:\n"
"FILTER_TYPE := { rsvp | u32 | bpf | fw | route | etc. }\n"
"FILTERID := ... format depends on classifier, see there\n"
"OPTIONS := ... try tc filter add <desired FILTER_KIND> help\n");
}
+static void chaintmplt_usage(void)
+{
+ fprintf(stderr,
+ "Usage: tc chaintemplate [ add | del | get | show ] [ dev STRING ]\n"
+ " tc chaintemplate [ add | del | get | show ] [ block BLOCK_INDEX ] ]\n");
+}
+
struct tc_filter_req {
struct nlmsghdr n;
struct tcmsg t;
@@ -85,7 +94,8 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
req->n.nlmsg_type = cmd;
req->t.tcm_family = AF_UNSPEC;
- if (cmd == RTM_NEWTFILTER && flags & NLM_F_CREATE)
+ if ((cmd == RTM_NEWTFILTER || cmd == RTM_NEWCHAINTMPLT) &&
+ flags & NLM_F_CREATE)
protocol = htons(ETH_P_ALL);
while (argc > 0) {
@@ -261,7 +271,10 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (n->nlmsg_type != RTM_NEWTFILTER &&
n->nlmsg_type != RTM_GETTFILTER &&
- n->nlmsg_type != RTM_DELTFILTER) {
+ n->nlmsg_type != RTM_DELTFILTER &&
+ n->nlmsg_type != RTM_NEWCHAINTMPLT &&
+ n->nlmsg_type != RTM_GETCHAINTMPLT &&
+ n->nlmsg_type != RTM_DELCHAINTMPLT) {
fprintf(stderr, "Not a filter(cmd %d)\n", n->nlmsg_type);
return 0;
}
@@ -280,20 +293,26 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
open_json_object(NULL);
- if (n->nlmsg_type == RTM_DELTFILTER)
+ if (n->nlmsg_type == RTM_DELTFILTER || n->nlmsg_type == RTM_DELCHAINTMPLT)
print_bool(PRINT_ANY, "deleted", "deleted ", true);
- if (n->nlmsg_type == RTM_NEWTFILTER &&
+ if ((n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAINTMPLT) &&
(n->nlmsg_flags & NLM_F_CREATE) &&
!(n->nlmsg_flags & NLM_F_EXCL))
print_bool(PRINT_ANY, "replaced", "replaced ", true);
- if (n->nlmsg_type == RTM_NEWTFILTER &&
+ if ((n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAINTMPLT) &&
(n->nlmsg_flags & NLM_F_CREATE) &&
(n->nlmsg_flags & NLM_F_EXCL))
print_bool(PRINT_ANY, "added", "added ", true);
- print_string(PRINT_FP, NULL, "filter ", NULL);
+ if (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER)
+ print_string(PRINT_FP, NULL, "filter ", NULL);
+ else
+ print_string(PRINT_FP, NULL, "chaintemplate ", NULL);
if (t->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
if (!filter_block_index ||
filter_block_index != t->tcm_block_index)
@@ -317,7 +336,9 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
}
}
- if (t->tcm_info) {
+ if (t->tcm_info && (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER ||
+ n->nlmsg_type == RTM_GETTFILTER)) {
f_proto = TC_H_MIN(t->tcm_info);
__u32 prio = TC_H_MAJ(t->tcm_info)>>16;
@@ -496,17 +517,19 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
argc--; argv++;
}
- if (!protocol_set) {
- fprintf(stderr, "Must specify filter protocol\n");
- return -1;
- }
+ if (cmd == RTM_GETTFILTER) {
+ if (!protocol_set) {
+ fprintf(stderr, "Must specify filter protocol\n");
+ return -1;
+ }
- if (!prio) {
- fprintf(stderr, "Must specify filter priority\n");
- return -1;
- }
+ if (!prio) {
+ fprintf(stderr, "Must specify filter priority\n");
+ return -1;
+ }
- req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+ req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+ }
if (chain_index_set)
addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
@@ -516,11 +539,13 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (k[0])
- addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
- else {
- fprintf(stderr, "Must specify filter type\n");
- return -1;
+ if (cmd == RTM_GETTFILTER) {
+ if (k[0])
+ addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
+ else {
+ fprintf(stderr, "Must specify filter type\n");
+ return -1;
+ }
}
if (d[0]) {
@@ -539,10 +564,11 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
+ if (cmd == RTM_GETTFILTER &&
+ q->parse_fopt(q, fhandle, argc, argv, &req.n))
return 1;
- if (!fhandle) {
+ if (!fhandle && cmd == RTM_GETTFILTER) {
fprintf(stderr, "Must specify filter \"handle\"\n");
return -1;
}
@@ -569,7 +595,7 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return 0;
}
-static int tc_filter_list(int argc, char **argv)
+static int tc_filter_list(int cmd, int argc, char **argv)
{
struct {
struct nlmsghdr n;
@@ -577,7 +603,7 @@ static int tc_filter_list(int argc, char **argv)
char buf[MAX_MSG];
} req = {
.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
- .n.nlmsg_type = RTM_GETTFILTER,
+ .n.nlmsg_type = cmd,
.t.tcm_parent = TC_H_UNSPEC,
.t.tcm_family = AF_UNSPEC,
};
@@ -725,7 +751,7 @@ static int tc_filter_list(int argc, char **argv)
int do_filter(int argc, char **argv, void *buf, size_t buflen)
{
if (argc < 1)
- return tc_filter_list(0, NULL);
+ return tc_filter_list(RTM_GETTFILTER, 0, NULL);
if (matches(*argv, "add") == 0)
return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
argc-1, argv+1, buf, buflen);
@@ -742,7 +768,7 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
return tc_filter_get(RTM_GETTFILTER, 0, argc-1, argv+1);
if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
|| matches(*argv, "lst") == 0)
- return tc_filter_list(argc-1, argv+1);
+ return tc_filter_list(RTM_GETTFILTER, argc-1, argv+1);
if (matches(*argv, "help") == 0) {
usage();
return 0;
@@ -751,3 +777,29 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
*argv);
return -1;
}
+
+int do_chaintmplt(int argc, char **argv, void *buf, size_t buflen)
+{
+ if (argc < 1)
+ return tc_filter_list(RTM_GETCHAINTMPLT, 0, NULL);
+ if (matches(*argv, "add") == 0) {
+ return tc_filter_modify(RTM_NEWCHAINTMPLT, NLM_F_EXCL | NLM_F_CREATE,
+ argc - 1, argv + 1, buf, buflen);
+ } else if (matches(*argv, "delete") == 0) {
+ return tc_filter_modify(RTM_DELCHAINTMPLT, 0,
+ argc - 1, argv + 1, buf, buflen);
+ } else if (matches(*argv, "get") == 0) {
+ return tc_filter_get(RTM_GETCHAINTMPLT, 0,
+ argc - 1, argv + 1);
+ } else if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0 ||
+ matches(*argv, "lst") == 0) {
+ return tc_filter_list(RTM_GETCHAINTMPLT, argc - 1, argv + 1);
+ } else if (matches(*argv, "help") == 0) {
+ chaintmplt_usage();
+ return 0;
+ }
+ fprintf(stderr, "Command \"%s\" is unknown, try \"tc chaintemplate help\".\n",
+ *argv);
+ return -1;
+}
+
diff --git a/tc/tc_monitor.c b/tc/tc_monitor.c
index 077b138d1ec5..33633837432c 100644
--- a/tc/tc_monitor.c
+++ b/tc/tc_monitor.c
@@ -43,7 +43,10 @@ static int accept_tcmsg(const struct sockaddr_nl *who,
if (timestamp)
print_timestamp(fp);
- if (n->nlmsg_type == RTM_NEWTFILTER || n->nlmsg_type == RTM_DELTFILTER) {
+ if (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAINTMPLT ||
+ n->nlmsg_type == RTM_DELCHAINTMPLT) {
print_filter(who, n, arg);
return 0;
}
--
2.14.4
^ permalink raw reply related
* [PATCH v3 net-next 5/5] net: emaclite: Remove unnecessary spaces
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <1530191510-10310-1-git-send-email-radhey.shyam.pandey@xilinx.com>
This patch fixes below checkpatch checks-
CHECK: spaces preferred around that '*' (ctx:VxV)
CHECK: No space is necessary after a cast
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes from v2:
- None
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 27 +++++++++++++------------
1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index f96c920..42f1f51 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -95,11 +95,11 @@
-#define TX_TIMEOUT (60*HZ) /* Tx timeout is 60 seconds. */
+#define TX_TIMEOUT (60 * HZ) /* Tx timeout is 60 seconds. */
#define ALIGNMENT 4
/* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */
-#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT)
+#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT)
#ifdef __BIG_ENDIAN
#define xemaclite_readl ioread32be
@@ -239,8 +239,8 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
/* Set up to output the remaining data */
align_buffer = 0;
- to_u8_ptr = (u8 *) &align_buffer;
- from_u8_ptr = (u8 *) from_u16_ptr;
+ to_u8_ptr = (u8 *)&align_buffer;
+ from_u8_ptr = (u8 *)from_u16_ptr;
/* Output the remaining data */
for (; length > 0; length--)
@@ -273,7 +273,7 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
u32 align_buffer;
from_u32_ptr = src_ptr;
- to_u16_ptr = (u16 *) dest_ptr;
+ to_u16_ptr = (u16 *)dest_ptr;
for (; length > 3; length -= 4) {
/* Copy each word into the temporary buffer */
@@ -289,9 +289,9 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
u8 *to_u8_ptr, *from_u8_ptr;
/* Set up to read the remaining data */
- to_u8_ptr = (u8 *) to_u16_ptr;
+ to_u8_ptr = (u8 *)to_u16_ptr;
align_buffer = *from_u32_ptr++;
- from_u8_ptr = (u8 *) &align_buffer;
+ from_u8_ptr = (u8 *)&align_buffer;
/* Read the remaining data */
for (; length > 0; length--)
@@ -351,7 +351,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
return -1; /* Buffer was full, return failure */
/* Write the frame to the buffer */
- xemaclite_aligned_write(data, (u32 __force *) addr, byte_count);
+ xemaclite_aligned_write(data, (u32 __force *)addr, byte_count);
xemaclite_writel((byte_count & XEL_TPLR_LENGTH_MASK),
addr + XEL_TPLR_OFFSET);
@@ -448,7 +448,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
length = maxlen;
/* Read from the EmacLite device */
- xemaclite_aligned_read((u32 __force *) (addr + XEL_RXBUFF_OFFSET),
+ xemaclite_aligned_read((u32 __force *)(addr + XEL_RXBUFF_OFFSET),
data, length);
/* Acknowledge the frame */
@@ -479,7 +479,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
/* Determine the expected Tx buffer address */
addr = drvdata->base_addr + drvdata->next_tx_buf_to_use;
- xemaclite_aligned_write(address_ptr, (u32 __force *) addr, ETH_ALEN);
+ xemaclite_aligned_write(address_ptr, (u32 __force *)addr, ETH_ALEN);
xemaclite_writel(ETH_ALEN, addr + XEL_TPLR_OFFSET);
@@ -572,10 +572,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
struct net_local *lp = netdev_priv(dev);
dev->stats.tx_packets++;
+
if (!lp->deferred_skb)
return;
- if (xemaclite_send_data(lp, (u8 *) lp->deferred_skb->data,
+ if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
lp->deferred_skb->len))
return;
@@ -620,7 +621,7 @@ static void xemaclite_rx_handler(struct net_device *dev)
skb_reserve(skb, 2);
- len = xemaclite_recv_data(lp, (u8 *) skb->data, len);
+ len = xemaclite_recv_data(lp, (u8 *)skb->data, len);
if (!len) {
dev->stats.rx_errors++;
@@ -1033,7 +1034,7 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
new_skb = orig_skb;
spin_lock_irqsave(&lp->reset_lock, flags);
- if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
+ if (xemaclite_send_data(lp, (u8 *)new_skb->data, len) != 0) {
/* If the Emaclite Tx buffer is busy, stop the Tx queue and
* defer the skb for transmission during the ISR, after the
* current transmission is complete
--
1.7.1
^ permalink raw reply related
* [PATCH v3 net-next 4/5] net: emaclite: Fix block comments style
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <1530191510-10310-1-git-send-email-radhey.shyam.pandey@xilinx.com>
This patch fixes below checkpatch warnings-
WARNING: Block comments use a trailing */ on a separate line
WARNING: Block comments use * on subsequent lines
WARNING: networking block comments don't use an empty /* line,
use /* Comment
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes from v2:
- None
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 34 +++++++++++++++---------
1 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index a17f0b6..f96c920 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -70,7 +70,8 @@
#define XEL_TSR_XMIT_IE_MASK 0x00000008 /* Tx interrupt enable bit */
#define XEL_TSR_XMIT_ACTIVE_MASK 0x80000000 /* Buffer is active, SW bit
* only. This is not documented
- * in the HW spec */
+ * in the HW spec
+ */
/* Define for programming the MAC address into the EmacLite */
#define XEL_TSR_PROG_MAC_ADDR (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_PROGRAM_MASK)
@@ -336,7 +337,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
drvdata->next_tx_buf_to_use ^= XEL_BUFFER_OFFSET;
} else if (drvdata->tx_ping_pong != 0) {
/* If the expected buffer is full, try the other buffer,
- * if it is configured in HW */
+ * if it is configured in HW
+ */
addr = (void __iomem __force *)((u32 __force)addr ^
XEL_BUFFER_OFFSET);
@@ -357,7 +359,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
/* Update the Tx Status Register to indicate that there is a
* frame to send. Set the XEL_TSR_XMIT_ACTIVE_MASK flag which
* is used by the interrupt handler to check whether a frame
- * has been transmitted */
+ * has been transmitted
+ */
reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
reg_data |= (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_XMIT_ACTIVE_MASK);
xemaclite_writel(reg_data, addr + XEL_TSR_OFFSET);
@@ -395,7 +398,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
/* The instance is out of sync, try other buffer if other
* buffer is configured, return 0 otherwise. If the instance is
* out of sync, do not update the 'next_rx_buf_to_use' since it
- * will correct on subsequent calls */
+ * will correct on subsequent calls
+ */
if (drvdata->rx_ping_pong != 0)
addr = (void __iomem __force *)((u32 __force)addr ^
XEL_BUFFER_OFFSET);
@@ -409,13 +413,15 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
return 0; /* No data was available */
}
- /* Get the protocol type of the ethernet frame that arrived */
+ /* Get the protocol type of the ethernet frame that arrived
+ */
proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
XEL_RPLR_LENGTH_MASK);
/* Check if received ethernet frame is a raw ethernet frame
- * or an IP packet or an ARP packet */
+ * or an IP packet or an ARP packet
+ */
if (proto_type > ETH_DATA_LEN) {
if (proto_type == ETH_P_IP) {
@@ -431,7 +437,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
length = XEL_ARP_PACKET_SIZE + ETH_HLEN + ETH_FCS_LEN;
else
/* Field contains type other than IP or ARP, use max
- * frame size and let user parse it */
+ * frame size and let user parse it
+ */
length = ETH_FRAME_LEN + ETH_FCS_LEN;
} else
/* Use the length in the frame, plus the header and trailer */
@@ -602,11 +609,11 @@ static void xemaclite_rx_handler(struct net_device *dev)
return;
}
- /*
- * A new skb should have the data halfword aligned, but this code is
+ /* A new skb should have the data halfword aligned, but this code is
* here just in case that isn't true. Calculate how many
* bytes we should reserve to get the data to start on a word
- * boundary */
+ * boundary
+ */
align = BUFFER_ALIGN(skb->data);
if (align)
skb_reserve(skb, align);
@@ -708,8 +715,8 @@ static int xemaclite_mdio_wait(struct net_local *lp)
unsigned long end = jiffies + 2;
/* wait for the MDIO interface to not be busy or timeout
- after some time.
- */
+ * after some time.
+ */
while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
XEL_MDIOCTRL_MDIOSTS_MASK) {
if (time_before_eq(end, jiffies)) {
@@ -1029,7 +1036,8 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
/* If the Emaclite Tx buffer is busy, stop the Tx queue and
* defer the skb for transmission during the ISR, after the
- * current transmission is complete */
+ * current transmission is complete
+ */
netif_stop_queue(dev);
lp->deferred_skb = new_skb;
/* Take the time stamp now, since we can't do this in an ISR. */
--
1.7.1
^ permalink raw reply related
* [PATCH v3 net-next 3/5] net: emaclite: update kernel-doc comments
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <1530191510-10310-1-git-send-email-radhey.shyam.pandey@xilinx.com>
This patch fixes below kernel-doc warnings:
Function parameter or member 'maxlen' not described in 'xemaclite_recv_data'
Function parameter or member 'address'not described in 'xemaclite_set_mac_address'
Excess function parameter 'addr' description in 'xemaclite_set_mac_address'
No description found for return value of 'xemaclite_interrupt'
No description found for return value of 'xemaclite_mdio_write'
Function parameter or member 'dev' not described in 'xemaclite_mdio_setup'
Excess function parameter 'ofdev' description in 'xemaclite_mdio_setup'
No description found for return value of 'xemaclite_open'
No description found for return value of 'xemaclite_close'
Excess function parameter 'match' description in 'xemaclite_of_probe'
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes from v2:
- None
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index b2c7afe..a17f0b6 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -369,6 +369,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
* xemaclite_recv_data - Receive a frame
* @drvdata: Pointer to the Emaclite device private data
* @data: Address where the data is to be received
+ * @maxlen: Maximum supported ethernet packet length
*
* This function is intended to be called from the interrupt context or
* with a wrapper which waits for the receive frame to be available.
@@ -488,7 +489,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
/**
* xemaclite_set_mac_address - Set the MAC address for this device
* @dev: Pointer to the network device instance
- * @addr: Void pointer to the sockaddr structure
+ * @address: Void pointer to the sockaddr structure
*
* This function copies the HW address from the sockaddr strucutre to the
* net_device structure and updates the address in HW.
@@ -638,6 +639,8 @@ static void xemaclite_rx_handler(struct net_device *dev)
* @dev_id: Void pointer to the network device instance used as callback
* reference
*
+ * Return: IRQ_HANDLED
+ *
* This function handles the Tx and Rx interrupts of the EmacLite device.
*/
static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
@@ -771,6 +774,8 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
*
* This function waits till the device is ready to accept a new MDIO
* request and then writes the val to the MDIO Write Data register.
+ *
+ * Return: 0 upon success or a negative error upon failure
*/
static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
u16 val)
@@ -804,7 +809,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
/**
* xemaclite_mdio_setup - Register mii_bus for the Emaclite device
* @lp: Pointer to the Emaclite device private data
- * @ofdev: Pointer to OF device structure
+ * @dev: Pointer to OF device structure
*
* This function enables MDIO bus in the Emaclite device and registers a
* mii_bus.
@@ -904,6 +909,9 @@ static void xemaclite_adjust_link(struct net_device *ndev)
* This function sets the MAC address, requests an IRQ and enables interrupts
* for the Emaclite device and starts the Tx queue.
* It also connects to the phy device, if MDIO is included in Emaclite device.
+ *
+ * Return: 0 on success. -ENODEV, if PHY cannot be connected.
+ * Non-zero error value on failure.
*/
static int xemaclite_open(struct net_device *dev)
{
@@ -974,6 +982,8 @@ static int xemaclite_open(struct net_device *dev)
* This function stops the Tx queue, disables interrupts and frees the IRQ for
* the Emaclite device.
* It also disconnects the phy device associated with the Emaclite device.
+ *
+ * Return: 0, always.
*/
static int xemaclite_close(struct net_device *dev)
{
@@ -1064,7 +1074,6 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
/**
* xemaclite_of_probe - Probe method for the Emaclite device.
* @ofdev: Pointer to OF device structure
- * @match: Pointer to the structure used for matching a device
*
* This function probes for the Emaclite device in the device tree.
* It initializes the driver data structure and the hardware, sets the MAC
--
1.7.1
^ permalink raw reply related
* [PATCH v3 net-next 2/5] net: emaclite: Simplify if-else statements
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1530191510-10310-1-git-send-email-radhey.shyam.pandey@xilinx.com>
Remove else as it is not required with if doing a return.
It also coalesce the format onto a single line and add the
missing space after the comma. Fixes below checkpatch warning-
WARNING: else is not generally useful after a break or return
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes from v2:
- Refactor to make failure path return early as suggested
by Joe Perches <joe@perches.com>
- Coalesce the format onto a single line as suggested by
by Joe Perches <joe@perches.com>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 34 +++++++++++-------------
1 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0544134..b2c7afe 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -564,19 +564,18 @@ static void xemaclite_tx_handler(struct net_device *dev)
struct net_local *lp = netdev_priv(dev);
dev->stats.tx_packets++;
- if (lp->deferred_skb) {
- if (xemaclite_send_data(lp,
- (u8 *) lp->deferred_skb->data,
- lp->deferred_skb->len) != 0)
- return;
- else {
- dev->stats.tx_bytes += lp->deferred_skb->len;
- dev_kfree_skb_irq(lp->deferred_skb);
- lp->deferred_skb = NULL;
- netif_trans_update(dev); /* prevent tx timeout */
- netif_wake_queue(dev);
- }
- }
+ if (!lp->deferred_skb)
+ return;
+
+ if (xemaclite_send_data(lp, (u8 *) lp->deferred_skb->data,
+ lp->deferred_skb->len))
+ return;
+
+ dev->stats.tx_bytes += lp->deferred_skb->len;
+ dev_kfree_skb_irq(lp->deferred_skb);
+ lp->deferred_skb = NULL;
+ netif_trans_update(dev); /* prevent tx timeout */
+ netif_wake_queue(dev);
}
/**
@@ -1052,13 +1051,12 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
{
u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
- if (p) {
- return (bool)*p;
- } else {
- dev_warn(&ofdev->dev, "Parameter %s not found,"
- "defaulting to false\n", s);
+ if (!p) {
+ dev_warn(&ofdev->dev, "Parameter %s not found, defaulting to false\n", s);
return false;
}
+
+ return (bool)*p;
}
static const struct net_device_ops xemaclite_netdev_ops;
--
1.7.1
^ permalink raw reply related
* [PATCH v3 net-next 1/5] net: emaclite: Use __func__ instead of hardcoded name
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <1530191510-10310-1-git-send-email-radhey.shyam.pandey@xilinx.com>
Switch hardcoded function name with a reference to __func__ making
the code more maintainable. Address below checkpatch warning:
WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
this function's name, in a string
+ "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
this function's name, in a string
+ "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes from v2:
- None
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 2a0c06e..0544134 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -757,7 +757,7 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
rc = xemaclite_readl(lp->base_addr + XEL_MDIORD_OFFSET);
dev_dbg(&lp->ndev->dev,
- "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
+ "%s(phy_id=%i, reg=%x) == %x\n", __func__,
phy_id, reg, rc);
return rc;
@@ -780,7 +780,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
u32 ctrl_reg;
dev_dbg(&lp->ndev->dev,
- "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
+ "%s(phy_id=%i, reg=%x, val=%x)\n", __func__,
phy_id, reg, val);
if (xemaclite_mdio_wait(lp))
--
1.7.1
^ permalink raw reply related
* [PATCH v3 net-next 0/5] Fixes coding style in xilinx_emaclite.c
From: Radhey Shyam Pandey @ 2018-06-28 13:11 UTC (permalink / raw)
To: davem, michal.simek, radhey.shyam.pandey, joe, andrew
Cc: netdev, linux-kernel, linux-arm-kernel
This patchset fixes checkpatch and kernel-doc warnings in
xilinx emaclite driver. No functional change.
Changes from v2:
-In 2/5 patch refactor if-else to make failure path return early.
-In 2/5 patch coalesce the format onto a single line and add the
missing space after the comma.
Radhey Shyam Pandey (5):
net: emaclite: Use __func__ instead of hardcoded name
net: emaclite: Simplify if-else statements
net: emaclite: update kernel-doc comments
net: emaclite: Fix block comments style
net: emaclite: Remove unnecessary spaces
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 112 ++++++++++++++-----------
1 files changed, 64 insertions(+), 48 deletions(-)
^ 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