On Fri, Feb 09, 2018 at 03:40:13PM +0100, Martin Pieuchot wrote:
> On 02/02/18(Fri) 12:18, Michele Curti wrote:
> > Hi,
> > in sys/dev/pci/if_em.c at line 469 there is:
> >     sc->hw.min_frame_size =
> >         ETHER_MIN_LEN + ETHER_CRC_LEN;
> > 
> > But ETHER_MIN_LEN already includes the CRC size:
> > #define ETHER_MIN_LEN   64      /* Minimum frame length, CRC included   */
> > 
> > In revision 1.41 the code changed in this way:
> >         sc->hw.min_frame_size =
> > -           MINIMUM_ETHERNET_PACKET_SIZE + ETHER_CRC_LEN;
> > +           ETHER_MIN_LEN + ETHER_CRC_LEN;
> > 
> > But MINIMUM_ETHERNET_PACKET_SIZE was the size without CRC.
> > 
> > #define MINIMUM_ETHERNET_FRAME_SIZE  64   /* With FCS */
> > #define ETHERNET_FCS_SIZE            4
> > #define MINIMUM_ETHERNET_PACKET_SIZE \
> >     (MINIMUM_ETHERNET_FRAME_SIZE - ETHERNET_FCS_SIZE)
> > 
> > So I attached the following diff to restore the original minimum 
> > size (by the way, what's the effect of having min_frame_size set 
> > to 68? Discarding valid packets sized from 64 to 67?)
> 
> I agree with your analyse.  How did you end up looking at this?  Does
> this fix a problem for you?

Hi Martin,
no, I had no problems.

I was reading "OpenBSD network stack internals" by Claudio Jeker, and 
I was just curious about it.  So I started reading the code from the 
interrupt routine and had trouble with the byte count.

The resolution led to this patch, but I haven't been able to understand 
if the patch may lead to regressions, despite the hints from other 
developers eheh...

Regards,
Michele

> 
> > Index: sys/dev/pci/if_em.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/if_em.c,v
> > retrieving revision 1.336
> > diff -u -p -r1.336 if_em.c
> > --- sys/dev/pci/if_em.c     25 Jul 2017 20:45:18 -0000      1.336
> > +++ sys/dev/pci/if_em.c     2 Feb 2018 11:13:43 -0000
> > @@ -466,8 +466,7 @@ em_attach(struct device *parent, struct 
> >                         MAX_JUMBO_FRAME_SIZE;
> >     }
> >  
> > -   sc->hw.min_frame_size = 
> > -       ETHER_MIN_LEN + ETHER_CRC_LEN;
> > +   sc->hw.min_frame_size = ETHER_MIN_LEN;
> >  
> >     /* Allocate Transmit Descriptor ring */
> >     if (em_dma_malloc(sc, sc->sc_tx_slots * sizeof(struct em_tx_desc),
> > 

Reply via email to