Hello,

On Tue, 2007-04-10 at 23:42 -0700, Pete Zaitcev wrote:
> > -   char flag_setup;
> > +   char flag_hdr;
> 
> I don't think this is fully compatible.

Why ?!? 'flag_hdr' is zero in exactly the same situation of old api. It
can assume more values that are interpreted by old code as signal of
setup header unavailability and in such situation the setup header is
effectively unavailable.

> >     unsigned int len_cap;   /* Delivered length */
> > -   unsigned char setup[SETUP_LEN]; /* Only for Control S-type */
> > +   union {
> > +           unsigned char setup[SETUP_LEN]; /* Only for Control S-type */
> > +           struct {
> > +                   int interval;
> > +                   unsigned int xfer_flags;
> > +           } compl_hdr;    /* For C/E-Type */
> > +           struct {
> > +                   int start_frame;
> > +                   int number_of_packets;
> > +           } iso_hdr;      /* For ISO P-Type */ 
> > +   };
> 
> The problem here is, xfer_flags are needed for both submission and
> callback. We know that they have to stay the same, but it's safer to
> have them available as is.

If and only if the xfer_flags is the only extended information that need
to be carried on both SUBMIT and COMPLETE event, it can be handled
somewhat with 48 bytes header. AFAIK the status field is significant
only for COMPLETE event (source: LDD 3rd edition), so with something
hackish like this:

struct mon_bin_hdr {
        u64 id;                 /* URB ID - from submission to callback */
        unsigned char type;     /* Same as in text API; extensible. */
        unsigned char xfer_type;        /* ISO, Intr, Control, Bulk */
        unsigned char epnum;    /* Endpoint number and transfer direction */
        unsigned char devnum;   /* Device address */
        unsigned short busnum;  /* Bus number */
        char flag_hdr;
        char flag_data;
        s64 ts_sec;             /* gettimeofday */
        s32 ts_usec;            /* gettimeofday */
        union {
                int status;     /* for C-type events */
                int xfer_flags; /* only for S-type events */
        };
        unsigned int len_urb;   /* Length of data (submitted or actual) */
        unsigned int len_cap;   /* Delivered length */
        union {
                unsigned char setup[SETUP_LEN]; /* Only for Control S-type */
                struct {
                        int interval;
                        unsigned int xfer_flags;
                } compl_hdr;    /* For C/E-Type */
                struct {
                        int start_frame;
                        int number_of_packets;
                } iso_hdr;      /* For ISO P-Type */
        };
};

all required information should be available. Someone could tell that
this is nasty, so I'm not going to bother you with a complete patch...

> In short, I would like to strong-arm you into accepting what I have. :-)

The only issue I have regards libpcap support for extended binary api:
changing the header length will require a new data link type and perhaps
obsoleting the 'old' one. I suppose I will find some resistance from
libpcap maintainers. 

Moreover it wold be nice to have some simply way to query the kernel for
the effective version of the usbmon binary api implementation, beside
checking the kernel version number (possible issue with non vanilla
kernel).

Anyway thanks again for the long and detailed review!

ciao,

Paolo

p.s. BTW I saw your submission for 'usbmon: bus zero' and  
and 'usbmon: Add class for binary interface'. 

I don't want to seam rude, but I hoped to get a 'Signed-off-by' in one
of them...

 
 
 --
 Email.it, the professional e-mail, gratis per te: http://www.email.it/f
 
 Sponsor:
 Prestiti Online. Scopri subito se sei finanziabile. in 24 ore senza spese né 
anticipi, clicca qui
* 
 Clicca qui: http://adv.email.it/cgi-bin/foclick.cgi?mid=2908&d=13-4

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to