Re: [Openlldp-users] Question about the bpf framer

[Terry S. <ter...@gm...>]

Hi Tom,

I suspect that when I was testing the FreeBSD code I didn't notice the extra
2 bytes... I did my original testing on Mac OS X PPC, which wasn't padded,
so the issues didn't appear for me.

Thanks for the patches and the insight on the BPF issues.  I'll get your
patches committed sometime within the next couple of days.

The reason you are seeing your own frames is due to to the following BPF
IOCTL:

BIOCSSEESENT
 BIOCGSEESENT   (u_int) Set or get the flag determining whether locally
generated packets on the interface should be returned by BPF. Set to zero to
see only incoming packets on the interface. Set to one to see packets
originating locally and remotely on the interface. This flag is initialized
to one by default.
I had this enabled by default to help me debug the BPF frame handler, but I
guess I forgot to disable it on the release tarball.  Try setting that flag
to 0 and see if it fixes the problem.

The interface problem you mentioned isn't fixed in CVS, so if you feel like
kicking us a patch for that we'd be happy to integrate it.

- Terry

On Jan 16, 2008 7:08 PM, Tom Judge <to...@to...> wrote:

> Terry Simons wrote:
> > Hi Tom,
> >
> > Yes, you are correct.
> >
> > That code is an attempt to remove the BPF header, which on the 32-bit
> > systems I've tested is 18 bytes, including 32-bit versions of FreeBSD.
> >
> > I never quite understood how the BPF layer worked with regards to that,
> > but it sounds like there's a difference in the header on 64-bit systems?
> >
> > I'm happy to look into this for you if you want, or if you have a patch
> > or some knowledge that could help us determine what's going on that
> > would be great too.
> >
> > - Terry
>
>
> Hi Terry,
>
> Thanks for your response.
>
> I took a quick look at this yesterday and found that on FreeBSD 6.2 i386
> sizeof(struct bpf_hdr) is 20 and on amd64 it is 32.  However from the
> bpf man page it seems that you cannot use the return value of
> sizeof(struct bpf_hdr) to remove the header.
>
>  From bpf(4):
> <quote>
>  The bh_hdrlen field exists to account for padding between the header
> and the link level protocol.  The purpose here is to guarantee proper
> alignment of the packet data structures, which is required on alignment
> sensitive architectures and improves performance on many other
> architectures. The packet filter insures that the bpf_hdr and the
> network layer header will be word aligned.  Suitable precautions must be
> taken when accessing the link layer protocol fields on alignment
> restricted machines.  (This is not a problem on an Ethernet, since the
> type field is a short falling on an even offset, and the addresses are
> probably accessed in a bytewise fashion).
> </quote>
>
> I have attached a patch (lldp_framer_fiX_read.patch) that reads the
> packet into a bpf_hdr malloc'd to  lldp_port->mtu (this should be
> renamed to mru (max recieve unit) maybe?).  Then uses bh_hdrlen and
> bh_caplen to extract the first from from the receive buffer into
> lldp_port->rx.frame.
>
> Also on FreeBSD it seemed a little silly to put the network card into
> promisc mode when we can just register to receive the multicast ethernet
> address on interface (affectively allowing us to program the mac filter
> on the NIC to allow the frames up the stack).  I have attached a patch
> that enables this (bpf_frammer_reg_multi.patch).
>
>
> One thing that I have just noticed is that we see our own frames (i.e.
> ones that we send) in the rx path.  This may be related to not putting
> the interface into promisc mode.  As I have only seen it on a box that
> was running the multicast code, also only on a i386 system.  If I cannot
> find out why this is happening I will add some checks to make the rx
> path drop packets that originated from the local tx path.
>
>
> Also it seems that the usage and semantics of iface_list in lldp_main.c
> are miss matched.  For example you can not add -i more than once because
> each time you specify it you overwrite the previous value.  Also the
> check for is the interface in the list is based on comparing the first
> value in the list which happens to be the only value.  I may be able to
> come up with a patch for this in the next few days or at the weekend, if
> this is not already fixed in CVS?
>
>
> Regards
>
> Tom
>
>
> Note:  The patches attached are generated against the 0.3alpha release.
>