Re: [clearview-discuss] libdlpi with libpcap

[sagun shakya <Sagun.Shakya () Sun COM>]

I've addressed all the comments from below and updated the webrev.

http://cr.opensolaris.org/~sagun/libpcap/

While testing the changes I found out that my changes didn't work on a Solaris system that had libdlpi.h version
that existed before the public libdlpi.h was integrated. Some changes had to be made to configure.in and I added a 
couple of new macros to aclocal.m4. Could you please review those changes as well.

thanks,

Sagun

sagun shakya wrote:
> Thank you for reviewing the changes Guy!
>
> FYI, I've also filed bug # 1882884 " Add libpcap support for new Solaris 
> features".
>
> Guy Harris wrote:
>   
> > > I mostly fixed the comment changes, thinking they were trivial and 
> > > left other Sun cstyle error alone. I can see how it is distracting 
> > > while reviewing  the actual changes. I've undone such changes and 
> > > posted a new webrev at:
> > >
> > > http://cr.opensolaris.org/~sagun/libpcap/
> > >       
> > pcap-streams.c isn't purely STREAMS-related, and most of the 
> > pcap-XXX.c files have implementations of the libpcap APIs; you might 
> > want to call it, for example, "dlpisubs.c", as it's subroutines for 
> > both the "raw DLPI" and libdlpi-mediated DLPI implementations.
> >     
> Accepted. I'll change pcap-streams.c pcap-dlpisubs.c.
>
>   
> > pcap_read_pkt() might be better named pcap_read_pkts(), or 
> > pcap_process_pkts() - it processes more than one packet.
> >     
> pcap_process_pkts fits more to the function. I've fixed it to be that.
>   
> > pcap_set_linktype() doesn't set the link-layer type in the sense that 
> > pcap_set_datalink() does - i.e., it doesn't change what the link-layer 
> > type of the packets supplied by the device is.  pcap_process_mactype() 
> > might be a better name.
> >     
> Accepted.
>   
> > pcap_conf_bufmod() also flushes the read side of the stream; you might 
> > want to do that outside that routine, as that's not related to setting 
> > up bufmod.  Then pcap_conf_bufmod() wouldn't exist on platforms 
> > without bufmod.
> >
> >     
> Yes, I had split pcap_alloc_databuf() for similar reason. Instead of 
> creating another routine I'll just do this exclusively in 
> pcap-[dlpi,libdlpi].c
>   
> > You might also want to have pcap_open_live() in pcap-dlpi.c set 
> > passive mode if available, so that libpcap-based applications can 
> > coexist with link aggregation even in pre-Solaris 11 versions.
> >     
> I'll add this to pcap_open_live in pcap_dlpi.c
>
> I'll send an updated webrev once I've updated these changes.
>   


--

Sagun Shakya
781.442.7344/ X27344
sagun.shakya () sun com


-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.