* [PATCH] USB: serial: add port statistics
@ 2019-09-18 9:14 yegorslists
2019-09-18 11:08 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: yegorslists @ 2019-09-18 9:14 UTC (permalink / raw)
To: linux-usb; +Cc: johan, gregkh, Yegor Yefremov
From: Yegor Yefremov <yegorslists@googlemail.com>
Add additional port statistics like received and transmitted bytes
the way /proc/tty/driver/serial does.
As usbserial driver already provides USB related information and
this line is longer than 100 characters, this patch adds an
additional line with the same port number:
0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
0: tx:112 rx:0
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index a3179fea38c8..154e65b6895f 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -42,6 +42,9 @@
#define USB_SERIAL_TTY_MAJOR 188
#define USB_SERIAL_TTY_MINORS 512 /* should be enough for a while */
+static int serial_get_icount(struct tty_struct *tty,
+ struct serial_icounter_struct *icount);
+
/* There is no MODULE_DEVICE_TABLE for usbserial.c. Instead
the MODULE_DEVICE_TABLE declarations in each serial driver
cause the "hotplug" program to pull in whatever module is necessary
@@ -471,10 +474,13 @@ static int serial_proc_show(struct seq_file *m, void *v)
seq_puts(m, "usbserinfo:1.0 driver:2.0\n");
for (i = 0; i < USB_SERIAL_TTY_MINORS; ++i) {
+ struct tty_struct *tty;
+ struct serial_icounter_struct icount;
port = usb_serial_port_get_by_minor(i);
if (port == NULL)
continue;
serial = port->serial;
+ tty = port->port.tty;
seq_printf(m, "%d:", i);
if (serial->type->driver.owner)
@@ -491,6 +497,22 @@ static int serial_proc_show(struct seq_file *m, void *v)
seq_printf(m, " path:%s", tmp);
seq_putc(m, '\n');
+ if (!serial_get_icount(tty, &icount)) {
+ seq_printf(m, "%d:", i);
+ seq_printf(m, " tx:%d rx:%d",
+ icount.tx, icount.rx);
+ if (icount.frame)
+ seq_printf(m, " fe:%d", icount.frame);
+ if (icount.parity)
+ seq_printf(m, " pe:%d", icount.parity);
+ if (icount.brk)
+ seq_printf(m, " brk:%d", icount.brk);
+ if (icount.overrun)
+ seq_printf(m, " oe:%d", icount.overrun);
+ if (icount.buf_overrun)
+ seq_printf(m, " bo:%d", icount.buf_overrun);
+ seq_putc(m, '\n');
+ }
usb_serial_put(serial);
mutex_unlock(&serial->disc_mutex);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] USB: serial: add port statistics
2019-09-18 9:14 [PATCH] USB: serial: add port statistics yegorslists
@ 2019-09-18 11:08 ` Greg KH
2019-09-18 11:22 ` Yegor Yefremov
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-09-18 11:08 UTC (permalink / raw)
To: yegorslists; +Cc: linux-usb, johan
On Wed, Sep 18, 2019 at 11:14:15AM +0200, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> Add additional port statistics like received and transmitted bytes
> the way /proc/tty/driver/serial does.
>
> As usbserial driver already provides USB related information and
> this line is longer than 100 characters, this patch adds an
> additional line with the same port number:
>
> 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
> 0: tx:112 rx:0
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
You can't change existing proc files without having the chance that
userspace tools will break.
Have you tried this and seen what dies a horrible death?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] USB: serial: add port statistics
2019-09-18 11:08 ` Greg KH
@ 2019-09-18 11:22 ` Yegor Yefremov
2019-09-18 11:45 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Yegor Yefremov @ 2019-09-18 11:22 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb, Johan Hovold
On Wed, Sep 18, 2019 at 1:08 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 18, 2019 at 11:14:15AM +0200, yegorslists@googlemail.com wrote:
> > From: Yegor Yefremov <yegorslists@googlemail.com>
> >
> > Add additional port statistics like received and transmitted bytes
> > the way /proc/tty/driver/serial does.
> >
> > As usbserial driver already provides USB related information and
> > this line is longer than 100 characters, this patch adds an
> > additional line with the same port number:
> >
> > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
> > 0: tx:112 rx:0
> >
> > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > ---
> > drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
>
> You can't change existing proc files without having the chance that
> userspace tools will break.
>
> Have you tried this and seen what dies a horrible death?
This patch is more a proof of concept (forgot to add RFC keyword). I
find statistics provdes by the 8250 driver very useful for debugging
purposes. What would be the best way to implemnt this feature for
usbserial driver?
a) extend current line:
0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...tx:112 rx:0
though this still can break parsing
b) creating special entries for FTDI and other UARTs? Though it would
be greate to have all usbserial UART handled the same way in the same
file
Best regards,
Yegor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] USB: serial: add port statistics
2019-09-18 11:22 ` Yegor Yefremov
@ 2019-09-18 11:45 ` Greg KH
2019-09-18 11:51 ` Yegor Yefremov
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-09-18 11:45 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: linux-usb, Johan Hovold
On Wed, Sep 18, 2019 at 01:22:42PM +0200, Yegor Yefremov wrote:
> On Wed, Sep 18, 2019 at 1:08 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Sep 18, 2019 at 11:14:15AM +0200, yegorslists@googlemail.com wrote:
> > > From: Yegor Yefremov <yegorslists@googlemail.com>
> > >
> > > Add additional port statistics like received and transmitted bytes
> > > the way /proc/tty/driver/serial does.
> > >
> > > As usbserial driver already provides USB related information and
> > > this line is longer than 100 characters, this patch adds an
> > > additional line with the same port number:
> > >
> > > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
> > > 0: tx:112 rx:0
> > >
> > > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > > ---
> > > drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> >
> > You can't change existing proc files without having the chance that
> > userspace tools will break.
> >
> > Have you tried this and seen what dies a horrible death?
>
> This patch is more a proof of concept (forgot to add RFC keyword). I
> find statistics provdes by the 8250 driver very useful for debugging
> purposes. What would be the best way to implemnt this feature for
> usbserial driver?
>
> a) extend current line:
>
> 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...tx:112 rx:0
>
> though this still can break parsing
>
> b) creating special entries for FTDI and other UARTs? Though it would
> be greate to have all usbserial UART handled the same way in the same
> file
Why is any of this needed at all? Also, be very aware of the security
issues involved here, we had to disable access of these values by
"normal" users for other tty devices, so please don't break that by
offering it up here again.
What is going to use this information?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] USB: serial: add port statistics
2019-09-18 11:45 ` Greg KH
@ 2019-09-18 11:51 ` Yegor Yefremov
2019-09-18 12:02 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Yegor Yefremov @ 2019-09-18 11:51 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb, Johan Hovold
On Wed, Sep 18, 2019 at 1:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 18, 2019 at 01:22:42PM +0200, Yegor Yefremov wrote:
> > On Wed, Sep 18, 2019 at 1:08 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Sep 18, 2019 at 11:14:15AM +0200, yegorslists@googlemail.com wrote:
> > > > From: Yegor Yefremov <yegorslists@googlemail.com>
> > > >
> > > > Add additional port statistics like received and transmitted bytes
> > > > the way /proc/tty/driver/serial does.
> > > >
> > > > As usbserial driver already provides USB related information and
> > > > this line is longer than 100 characters, this patch adds an
> > > > additional line with the same port number:
> > > >
> > > > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
> > > > 0: tx:112 rx:0
> > > >
> > > > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > > > ---
> > > > drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
> > > > 1 file changed, 22 insertions(+)
> > >
> > > You can't change existing proc files without having the chance that
> > > userspace tools will break.
> > >
> > > Have you tried this and seen what dies a horrible death?
> >
> > This patch is more a proof of concept (forgot to add RFC keyword). I
> > find statistics provdes by the 8250 driver very useful for debugging
> > purposes. What would be the best way to implemnt this feature for
> > usbserial driver?
> >
> > a) extend current line:
> >
> > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...tx:112 rx:0
> >
> > though this still can break parsing
> >
> > b) creating special entries for FTDI and other UARTs? Though it would
> > be greate to have all usbserial UART handled the same way in the same
> > file
>
> Why is any of this needed at all? Also, be very aware of the security
> issues involved here, we had to disable access of these values by
> "normal" users for other tty devices, so please don't break that by
> offering it up here again.
>
> What is going to use this information?
This feature is not a "must have" one but it is convenient to see
transferred/received bytes and error flags from user space. If some
serial software is not working like expected and doesn't provide
enough debugging information one can quickly look at port statistics
from the console in order to check whether and how many bytes were
transferred or whether the were some communication errors.
Yegor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] USB: serial: add port statistics
2019-09-18 11:51 ` Yegor Yefremov
@ 2019-09-18 12:02 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-09-18 12:02 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: linux-usb, Johan Hovold
On Wed, Sep 18, 2019 at 01:51:29PM +0200, Yegor Yefremov wrote:
> On Wed, Sep 18, 2019 at 1:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Sep 18, 2019 at 01:22:42PM +0200, Yegor Yefremov wrote:
> > > On Wed, Sep 18, 2019 at 1:08 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Sep 18, 2019 at 11:14:15AM +0200, yegorslists@googlemail.com wrote:
> > > > > From: Yegor Yefremov <yegorslists@googlemail.com>
> > > > >
> > > > > Add additional port statistics like received and transmitted bytes
> > > > > the way /proc/tty/driver/serial does.
> > > > >
> > > > > As usbserial driver already provides USB related information and
> > > > > this line is longer than 100 characters, this patch adds an
> > > > > additional line with the same port number:
> > > > >
> > > > > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...
> > > > > 0: tx:112 rx:0
> > > > >
> > > > > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > > > > ---
> > > > > drivers/usb/serial/usb-serial.c | 22 ++++++++++++++++++++++
> > > > > 1 file changed, 22 insertions(+)
> > > >
> > > > You can't change existing proc files without having the chance that
> > > > userspace tools will break.
> > > >
> > > > Have you tried this and seen what dies a horrible death?
> > >
> > > This patch is more a proof of concept (forgot to add RFC keyword). I
> > > find statistics provdes by the 8250 driver very useful for debugging
> > > purposes. What would be the best way to implemnt this feature for
> > > usbserial driver?
> > >
> > > a) extend current line:
> > >
> > > 0: module:ftdi_sio name:"FTDI USB Serial Device" vendor:0403 ...tx:112 rx:0
> > >
> > > though this still can break parsing
> > >
> > > b) creating special entries for FTDI and other UARTs? Though it would
> > > be greate to have all usbserial UART handled the same way in the same
> > > file
> >
> > Why is any of this needed at all? Also, be very aware of the security
> > issues involved here, we had to disable access of these values by
> > "normal" users for other tty devices, so please don't break that by
> > offering it up here again.
> >
> > What is going to use this information?
>
> This feature is not a "must have" one but it is convenient to see
> transferred/received bytes and error flags from user space. If some
> serial software is not working like expected and doesn't provide
> enough debugging information one can quickly look at port statistics
> from the console in order to check whether and how many bytes were
> transferred or whether the were some communication errors.
Again, it's a security issue, so be careful about this. If you _REALLY_
need it, make it a debugfs file, readble by root only.
Or a tracepoint, and then you can have a userspace read the data using
ebpf :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-18 12:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-18 9:14 [PATCH] USB: serial: add port statistics yegorslists
2019-09-18 11:08 ` Greg KH
2019-09-18 11:22 ` Yegor Yefremov
2019-09-18 11:45 ` Greg KH
2019-09-18 11:51 ` Yegor Yefremov
2019-09-18 12:02 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).