Re: svn commit: r304584 - head/sys/dev/alc

2016-08-21 Thread YongHyeon PYUN
On Mon, Aug 22, 2016 at 03:28:06AM +, Pyun YongHyeon wrote:
> Author: yongari
> Date: Mon Aug 22 03:28:06 2016
> New Revision: 304584
> URL: https://svnweb.freebsd.org/changeset/base/304584
> 
> Log:
>   Add a missing change in r304575.
>   
>   Noticed by: jhb

Actually it was pointed out by markj. Sorry.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r304439 - head/sys/dev/usb/net

2016-08-19 Thread YongHyeon PYUN
On Fri, Aug 19, 2016 at 12:39:58PM +0200, Hans Petter Selasky wrote:
> On 08/19/16 11:22, YongHyeon PYUN wrote:
> >On Fri, Aug 19, 2016 at 11:11:56AM +0200, Hans Petter Selasky wrote:
> >>On 08/19/16 10:55, YongHyeon PYUN wrote:
> >>>I think the order is right but it was not tested on big-endian
> >>>systems.
> >>
> >>Hi,
> >>
> >>I'm pretty sure the ifdef is wrong, because you write the fields one at
> >>a time, using htole32():
> >>
> >>txhdr.mss = 0;
> >>txhdr.len = 
> >>htole32(AXGE_TXBYTES(m->m_pkthdr.len));
> >>
> >>Big endian machines don't re-order variables like this.
> >>
> >>You should remove the #else part.
> >
> >Wouldn't USB stack pass txhdr structure without any
> >modification? And controller want to see len (low 32bits address)
> >first and then mss (high 32bits address). On big endian systems I
> >guess this should be reversed in host memory layout.  This is so
> >confusing so I could be wrong.
> 
> The USB stack passes TXHDR as-is and the host controller is byte 
> oriented, not 64-bit word oriented. That's why the layout is the same as 
> long as you assign per 32-bit field.
> 

Ok, fixed in r304458.
Thanks for pointing it out!
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r304439 - head/sys/dev/usb/net

2016-08-19 Thread YongHyeon PYUN
On Fri, Aug 19, 2016 at 11:11:56AM +0200, Hans Petter Selasky wrote:
> On 08/19/16 10:55, YongHyeon PYUN wrote:
> >I think the order is right but it was not tested on big-endian
> >systems.
> 
> Hi,
> 
> I'm pretty sure the ifdef is wrong, because you write the fields one at 
> a time, using htole32():
> 
> txhdr.mss = 0;
> txhdr.len = htole32(AXGE_TXBYTES(m->m_pkthdr.len));
> 
> Big endian machines don't re-order variables like this.
> 
> You should remove the #else part.

Wouldn't USB stack pass txhdr structure without any
modification? And controller want to see len (low 32bits address)
first and then mss (high 32bits address). On big endian systems I
guess this should be reversed in host memory layout.  This is so
confusing so I could be wrong.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r304439 - head/sys/dev/usb/net

2016-08-19 Thread YongHyeon PYUN
On Fri, Aug 19, 2016 at 10:50:47AM +0200, Hans Petter Selasky wrote:
> On 08/19/16 02:50, Pyun YongHyeon wrote:
> >Modified: head/sys/dev/usb/net/if_axgereg.h
> >==
> >--- head/sys/dev/usb/net/if_axgereg.hFri Aug 19 00:03:41 2016 
> >(r304438)
> >+++ head/sys/dev/usb/net/if_axgereg.hFri Aug 19 00:50:32 2016 
> >(r304439)
> >@@ -156,19 +156,20 @@ enum {
> > struct axge_frame_txhdr {
> > #if BYTE_ORDER == LITTLE_ENDIAN
> > uint32_tlen;
> >-#define AXGE_TXLEN_MASK 0x0001
> >-#define AXGE_VLAN_INSERT0x2000
> >-#define AXGE_CSUM_DISABLE   0x8000
> > uint32_tmss;
> >-#define AXGE_MSS_MASK   0x3FFF
> >-#define AXGE_PADDING0x80008000
> >-#define AXGE_VLAN_TAG_MASK  0x
> > #else
> > uint32_tmss;
> > uint32_tlen;
> > #endif
> > } __packed;
> >
> 
> Hi,
> 
> Is it correct to switch the order of mss and len variables for 
> bit/little endian? Looks buggy to me.
> 

I think the order is right but it was not tested on big-endian
systems.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r304326 - head/sys/dev/usb/net

2016-08-18 Thread YongHyeon PYUN
On Thu, Aug 18, 2016 at 04:44:29PM +, Bjoern A. Zeeb wrote:
> On 18 Aug 2016, at 5:07, Pyun YongHyeon wrote:
> 
> >Author: yongari
> >Date: Thu Aug 18 05:07:02 2016
> >New Revision: 304326
> >URL: https://svnweb.freebsd.org/changeset/base/304326
> >

[...]

> >+struct axge_frame_txhdr {
> >+#if BYTE_ORDER == LITTLE_ENDIAN
> >+uint32_tlen;
> >+#define AXGE_TXLEN_MASK 0x0001
> >+#define AXGE_VLAN_INSERT0x2000
> >+#define AXGE_CSUM_DISABLE   0x8000
> >+uint32_tmss;
> >+#define AXGE_MSS_MASK   0x3FFF
> >+#define AXGE_PADDING0x80008000
> >+#define AXGE_VLAN_TAG_MASK  0x
> >+#else
> >+uint32_tmss;
> >+uint32_tlen;
> >+#endif
> >+} __packed;
> >+
> >+#define AXGE_TXBYTES(x) ((x) & AXGE_TXLEN_MASK)
> 
> 
> AXGE_TXLEN_MASK is only defined for LITTLE_ENDIAN and thus breaks builds 
> on others.
> 
> AXGE_CSUM_DISABLE as well ..
> 

Oops, fixed in r304439.
Thanks.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r266974 - in head/sys: dev/dc dev/fxp dev/mii dev/netmap kern net

2014-06-02 Thread Yonghyeon PYUN
On Mon, Jun 02, 2014 at 11:42:10AM -0700, Marcel Moolenaar wrote:
 
 On Jun 2, 2014, at 11:27 AM, Adrian Chadd adr...@freebsd.org wrote:
 
  .. and actually, bikeshedding for a moment, would we be able to move a
  lot of these accessor methods over to inlines? Would that break the
  Juniper way of doing things?
 
 That would definitely break Juniper as it doesn't give a stable
 ABI.
 
 I've suggested an approach that allows for both, but it was deemed
 unnecessary. The argument being that the function call overhead is
 negligible.
 
 We can always revisit that decision if needed...
 

The function call overheads shall show measurable differences on
slow boxes.  This change adds several function calls in driver's
fast path(interrupt handler, packet statistics, checksum
offloading checking and etc) and these functions would be called
on every TX/RX packet.
It would be great if there is a way to minimize function call
overheads in fast path.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r263957 - in head/sys: dev/age dev/alc dev/ale dev/bce dev/bge dev/fxp dev/jme dev/msk dev/nfe dev/sge pci

2014-03-30 Thread Yonghyeon PYUN
On Sun, Mar 30, 2014 at 09:00:59PM -0700, John-Mark Gurney wrote:
 Pyun YongHyeon wrote this message on Mon, Mar 31, 2014 at 01:54 +:
  Author: yongari
  Date: Mon Mar 31 01:54:59 2014
  New Revision: 263957
  URL: http://svnweb.freebsd.org/changeset/base/263957
  
  Log:
Increase the number of TX DMA segments from 32 to 35.  It turned
out 32 is not enough to support a full sized TSO packet.
While I'm here fix a long standing bug introduced in r169632 in
bce(4) where it didn't include L2 header length of TSO packet in
the maximum DMA segment size calculation.
 
 I assume all of the hardware supports this increase?
 

Yes.  Data sheet does not mention about such limitation.  txp(4)
has a limitation on the number of TX segments but I didn't
implement TSO in txp(4).

 Also, is there a reason to only increase up to 35 and not something
 larger, like 64?  Is there a memory or performance penalty?
 

If 64 does not overflow kernel stack we can also bump the number to
64.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r263957 - in head/sys: dev/age dev/alc dev/ale dev/bce dev/bge dev/fxp dev/jme dev/msk dev/nfe dev/sge pci

2014-03-30 Thread Yonghyeon PYUN
On Mon, Mar 31, 2014 at 01:25:35PM +0900, Yonghyeon PYUN wrote:
 On Sun, Mar 30, 2014 at 09:00:59PM -0700, John-Mark Gurney wrote:
  Pyun YongHyeon wrote this message on Mon, Mar 31, 2014 at 01:54 +:
   Author: yongari
   Date: Mon Mar 31 01:54:59 2014
   New Revision: 263957
   URL: http://svnweb.freebsd.org/changeset/base/263957
   
   Log:
 Increase the number of TX DMA segments from 32 to 35.  It turned
 out 32 is not enough to support a full sized TSO packet.
 While I'm here fix a long standing bug introduced in r169632 in
 bce(4) where it didn't include L2 header length of TSO packet in
 the maximum DMA segment size calculation.
  
  I assume all of the hardware supports this increase?
  
 
 Yes.  Data sheet does not mention about such limitation.  txp(4)
 has a limitation on the number of TX segments but I didn't
 implement TSO in txp(4).
 
  Also, is there a reason to only increase up to 35 and not something
  larger, like 64?  Is there a memory or performance penalty?
  
 
 If 64 does not overflow kernel stack we can also bump the number to
 64.

Hmm, I think I didn't answer on performance penalty. If hardware
allows only a single outstanding DMA read operation, having
multiple TX buffers would result in lower performance.  However
it's common to have multiple TX buffers in TSO so I don't think it
could change performance number in TSO path.  And I think most high
end server NICs support multiple outstanding DMA read operation.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r260224 - head/sys/netinet

2014-01-05 Thread Yonghyeon PYUN
On Fri, Jan 03, 2014 at 11:03:12AM +, Gleb Smirnoff wrote:
 Author: glebius
 Date: Fri Jan  3 11:03:12 2014
 New Revision: 260224
 URL: http://svnweb.freebsd.org/changeset/base/260224
 
 Log:
   Make failure of ifpromisc() a non-fatal error. This makes it possible to
   run carp(4) on vtnet(4).
   

vtnet(4) is the only device that doesn't correctly support
promiscuous mode?  I don't know details of vtnet(4) but it seems
it's not hard to mimic promiscuous mode.  I'm not sure why the
driver returns ENOTSUP to user land given that vtnet(4) defaults
to promiscuous mode for backwards compatibility.  It also does
not handle multicast filter configuration when VTNET_FLAG_CTRL_RX
flag is not set.  If vtnet(4) does not support multicast filter,
it shouldn't announce IFF_MULTICAST. I wonder how vtnet(4) can work
with carp(4) given that its multicast handling is ignored.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r242425 - head/sys/dev/ti

2012-11-01 Thread YongHyeon PYUN
On Wed, Oct 31, 2012 at 11:16:43PM -0700, Adrian Chadd wrote:
 so where'd this happen? interleaved TX nowdays via if_transmit() ?
 

if_output after fragmentation.

 
 
 Adrian
 
 On 31 October 2012 22:39, Pyun YongHyeon yong...@freebsd.org wrote:
  Author: yongari
  Date: Thu Nov  1 05:39:21 2012
  New Revision: 242425
  URL: http://svn.freebsd.org/changeset/base/242425
 
  Log:
Remove TCP/UDP checksum offloading feature for IP fragmented
datagrams.  Traditionally upper stack fragmented packets without
computing TCP/UDP checksum and these datagrams were passed to
driver.  But there are chances that other packets slip into the
interface queue in SMP world. If this happens firmware running on
MIPS 4000 processor in the controller would see mixed packets and
it shall send out corrupted packets.
While I'm here simplify checksum offloading setup.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf

2012-10-31 Thread YongHyeon PYUN
On Wed, Oct 31, 2012 at 08:59:06AM +0100, Andre Oppermann wrote:
 On 30.10.2012 03:25, YongHyeon PYUN wrote:
 On Mon, Oct 29, 2012 at 09:20:59AM +0100, Andre Oppermann wrote:
 On 29.10.2012 22:40, YongHyeon PYUN wrote:
 On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote:
 On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote:
 Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote:
 Y  On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote:
 Y  A On 26.10.2012 23:06, Gleb Smirnoff wrote:
 Y  A  Author: glebius
 Y  A  Date: Fri Oct 26 21:06:33 2012
 Y  A  New Revision: 242161
 Y  A  URL: http://svn.freebsd.org/changeset/base/242161
 Y  A 
 Y  A  Log:
 Y  A o Remove last argument to ip_fragment(), and obtain all
 needed information
 Y  A   on checksums directly from mbuf flags. This simplifies
 code.
 Y  A o Clear CSUM_IP from the mbuf in ip_fragment() if we did
 checksums in
 Y
 Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented
 Y packets(CSUM_IP_FRAGS) still works after this change.  ti(4)
 Y requires CSUM_IP should be set for IP fragmented packets. Not sure
 Y whether it's a bug or not. I have a ti(4) controller but I don't
 Y remember where I can find it and don't have a link
 Y parter(1000baseSX) to test it. :-(
 
 ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do
 
 Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4)
 is the only controller that supports TCP/UDP checksum offloading
 for an IP fragmented packet.
 
 This is a bit weird if it doesn't do the fragmentation itself.
 Computing the IP header checksum doesn't differ for normal and
 fragmented packets.  The protocol checksum (TCP or UDP) stays
 the same for in the case of IP level fragmentation.  It is only
 visible in the first fragment which includes the protocol header.
 
 My interpretation for CSUM_IP_FRAGS works like the following.
   - Only peuso header checksum for TCP/UDP is computed by upper
 stack.
   - Controller has no ability to fragment the packet so it should
 done in upper stack(i.e. ip_output()).
   - When ip_output() has to fragment the packet, it just fragments
 the packet without completing TCP/UDP and IP checksum. If
 controller does not support CSUM_IP_FRAGS feature, ip_output()
 can't delay TCP/UDP checksum in this stage.
   - The fragmented packets are sent to driver. Driver sets
 appropriate bits of DMA descriptor based on fragmentation field
 of mbuf(M_FRAG, M_LASTFRAG) and issue the frame to controller.
   - The firmware of controller queues the fragmented frames up in
 its internal memory and hold off sending out the frames since it
 has to compute TCP/UDP checksum. When it sees a frame which
 indicates the end of fragmented frame it finally computes
 TCP/UDP checksum and send each frame out to wire by computing
 IP checksum on the fly.
 The difference is which one(upper stack vs. controller) computes
 TCP/UDP/IP checksum.
 
 Such a behavior doesn't make much sense and probably wasn't used at all
 in practice.  It's very complex as well.  Plus you can't guarantee that

It's job of firmware running on embedded MIPS R4000 in the
controller. ti(4) was one of the best smart controller in the past
and it even supported header split!

 there won't be other packet slipping into the interface queue in an SMP
 world.
 

Hmm, right, I should have noticed that after FreeBSD's removal for
splnet()/splimp(). Since ti(4) is the only controller that supports
TCP/UDP checksum offloading on IP fragmented packets and it's rare
to see ti(4) controllers in these days, I think there is no much
point to make the hardware feature work. I'll remove the feature
in ti(4).
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf

2012-10-29 Thread YongHyeon PYUN
On Mon, Oct 29, 2012 at 09:20:59AM +0100, Andre Oppermann wrote:
 On 29.10.2012 22:40, YongHyeon PYUN wrote:
 On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote:
 On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote:
 Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote:
 Y  On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote:
 Y  A On 26.10.2012 23:06, Gleb Smirnoff wrote:
 Y  A  Author: glebius
 Y  A  Date: Fri Oct 26 21:06:33 2012
 Y  A  New Revision: 242161
 Y  A  URL: http://svn.freebsd.org/changeset/base/242161
 Y  A 
 Y  A  Log:
 Y  A o Remove last argument to ip_fragment(), and obtain all 
 needed information
 Y  A   on checksums directly from mbuf flags. This simplifies 
 code.
 Y  A o Clear CSUM_IP from the mbuf in ip_fragment() if we did 
 checksums in
 Y
 Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented
 Y packets(CSUM_IP_FRAGS) still works after this change.  ti(4)
 Y requires CSUM_IP should be set for IP fragmented packets. Not sure
 Y whether it's a bug or not. I have a ti(4) controller but I don't
 Y remember where I can find it and don't have a link
 Y parter(1000baseSX) to test it. :-(
 
 ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do
 
 Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4)
 is the only controller that supports TCP/UDP checksum offloading
 for an IP fragmented packet.
 
 This is a bit weird if it doesn't do the fragmentation itself.
 Computing the IP header checksum doesn't differ for normal and
 fragmented packets.  The protocol checksum (TCP or UDP) stays
 the same for in the case of IP level fragmentation.  It is only
 visible in the first fragment which includes the protocol header.

My interpretation for CSUM_IP_FRAGS works like the following.
 - Only peuso header checksum for TCP/UDP is computed by upper
   stack.
 - Controller has no ability to fragment the packet so it should
   done in upper stack(i.e. ip_output()).
 - When ip_output() has to fragment the packet, it just fragments
   the packet without completing TCP/UDP and IP checksum. If
   controller does not support CSUM_IP_FRAGS feature, ip_output()
   can't delay TCP/UDP checksum in this stage.
 - The fragmented packets are sent to driver. Driver sets
   appropriate bits of DMA descriptor based on fragmentation field
   of mbuf(M_FRAG, M_LASTFRAG) and issue the frame to controller.
 - The firmware of controller queues the fragmented frames up in
   its internal memory and hold off sending out the frames since it
   has to compute TCP/UDP checksum. When it sees a frame which
   indicates the end of fragmented frame it finally computes
   TCP/UDP checksum and send each frame out to wire by computing
   IP checksum on the fly.
The difference is which one(upper stack vs. controller) computes
TCP/UDP/IP checksum.

 
 software checksums, and thus won't clear these flags.
 
 Potentially a driver that announces one flag in if_hwassist but relies on
 couple of flags to be set on mbuf is not correct. If a driver can't do 
 single
 checksum  processing independently from others, then it should set or 
 clear
 appropriate flags in if_hwassist as a group.
 
 Hmm, then what would be best way to achieve CSUM_IP_FRAGS in
 driver? I don't have clear idea how to utilize the hardware
 feature. The stack should tell that the mbuf needs TCP/UDP checksum
 offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by
 upper stack).
 
 As I said there can't be fragment checksumming without hardware

It's up to controller's firmware. It does not send the fragmented
frame until it computes TCP/UDP checksum.

 based fragmentation.  We have three cases here:
 
  1. TSO where the hardware does the segmentation, TCP and IP header
 checksums for each generated packet.
  2. IP packet fragmentation where a packet is split, the IP header
 checksum is recomputed for each fragment, but the protocol csum
 stays the same and is not modified.
  3. UDP fragmentation where a large packet is sent to the hardware
 and it generates first the UDP checksum and then splits it into
 IP fragments each with its own IP header checksum.
 
 So we end up with these possible large send hardware offload capabilities:
  TSO: including IPv4hdr and TCP checksumming
  UDP fragmentation: including IPv4hdr and UDP checksumming
  IP fragmentation: including IPv4hdr checksumming
 
 Besides that we have the packet = MTU sized offload capabilities:
  TCP checksumming
  UDP checksumming
  SCTP checksumming
  IPv4hdr checksumming
 
 Y  A   hardware. Some driver may not announce CSUM_IP in theur 
 if_hwassist,
 
 
 Oh, that was a typo! Software was meant.
 
 That explains quite a bit of confusion.
 
 -- 
 Andre
 
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr

Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf

2012-10-28 Thread YongHyeon PYUN
On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote:
 On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote:
 A On 26.10.2012 23:06, Gleb Smirnoff wrote:
 A  Author: glebius
 A  Date: Fri Oct 26 21:06:33 2012
 A  New Revision: 242161
 A  URL: http://svn.freebsd.org/changeset/base/242161
 A 
 A  Log:
 A o Remove last argument to ip_fragment(), and obtain all needed 
 information
 A   on checksums directly from mbuf flags. This simplifies code.
 A o Clear CSUM_IP from the mbuf in ip_fragment() if we did checksums in

I'm not sure whether ti(4)'s checksum offloading for IP fragmented
packets(CSUM_IP_FRAGS) still works after this change.  ti(4)
requires CSUM_IP should be set for IP fragmented packets. Not sure
whether it's a bug or not. I have a ti(4) controller but I don't
remember where I can find it and don't have a link
parter(1000baseSX) to test it. :-(

 A   hardware. Some driver may not announce CSUM_IP in theur if_hwassist,
 A   although try to do checksums if CSUM_IP set on mbuf. Example is 
 em(4).

em(4) had TX IP checksum offloading support but it was removed
without justification. There could be some reason on that decision
but I don't see any compelling reason.

 A 
 A I'm not getting your description here?  Why work around a bug in a driver
 A in ip_fragment() when we can fix the bug in the driver?
 
 Well, that was actually bug in the stack and a very special driver that
 demonstrates it. I may even agree that driver is incorrect, but the stack was
 incorrect, too.
 
 -- 
 Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r242161 - in head/sys: net netinet netpfil/pf

2012-10-28 Thread YongHyeon PYUN
On Mon, Oct 29, 2012 at 09:21:00AM +0400, Gleb Smirnoff wrote:
 On Mon, Oct 29, 2012 at 01:41:04PM -0700, YongHyeon PYUN wrote:
 Y On Sun, Oct 28, 2012 at 02:01:37AM +0400, Gleb Smirnoff wrote:
 Y  On Sat, Oct 27, 2012 at 12:58:52PM +0200, Andre Oppermann wrote:
 Y  A On 26.10.2012 23:06, Gleb Smirnoff wrote:
 Y  A  Author: glebius
 Y  A  Date: Fri Oct 26 21:06:33 2012
 Y  A  New Revision: 242161
 Y  A  URL: http://svn.freebsd.org/changeset/base/242161
 Y  A 
 Y  A  Log:
 Y  A o Remove last argument to ip_fragment(), and obtain all needed 
 information
 Y  A   on checksums directly from mbuf flags. This simplifies code.
 Y  A o Clear CSUM_IP from the mbuf in ip_fragment() if we did 
 checksums in
 Y 
 Y I'm not sure whether ti(4)'s checksum offloading for IP fragmented
 Y packets(CSUM_IP_FRAGS) still works after this change.  ti(4)
 Y requires CSUM_IP should be set for IP fragmented packets. Not sure
 Y whether it's a bug or not. I have a ti(4) controller but I don't
 Y remember where I can find it and don't have a link
 Y parter(1000baseSX) to test it. :-(
 
 ti(4) declares both CSUM_IP and CSUM_IP_FRAGS, so ip_fragment() won't do

Because it supports both CSUM_IP and CSUM_IP_FRAGS. Probably ti(4)
is the only controller that supports TCP/UDP checksum offloading
for an IP fragmented packet.

 software checksums, and thus won't clear these flags.
 
 Potentially a driver that announces one flag in if_hwassist but relies on
 couple of flags to be set on mbuf is not correct. If a driver can't do single
 checksum  processing independently from others, then it should set or clear
 appropriate flags in if_hwassist as a group.

Hmm, then what would be best way to achieve CSUM_IP_FRAGS in
driver? I don't have clear idea how to utilize the hardware
feature. The stack should tell that the mbuf needs TCP/UDP checksum
offloading for IP fragmented packet(i.e. CSUM_IP_FRAGS is not set by
upper stack).

 
 Y  A   hardware. Some driver may not announce CSUM_IP in theur 
 if_hwassist,

 
 Oh, that was a typo! Software was meant.
 
 -- 
 Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r230286 - head/sys/dev/bge

2012-01-18 Thread YongHyeon PYUN
On Wed, Jan 18, 2012 at 09:53:51AM -0500, John Baldwin wrote:
 On Tuesday, January 17, 2012 5:15:33 pm Pyun YongHyeon wrote:
  Author: yongari
  Date: Tue Jan 17 22:15:33 2012
  New Revision: 230286
  URL: http://svn.freebsd.org/changeset/base/230286
  
  Log:
Introduce a tunable that disables use of MSI.
Non-zero value will use INTx.
 
 Hmm, do you think it is best to do this on a per-device level vs a per-driver 
 level (e.g. a 'hw.driver.msi' tunable ala mfi(4))?  Also, I think it is 

I thought that too. But what if other bge(4) controller on the box
works with MSI? I admit it would be rare case but making it
per-device-level wouldn't hurt.

 better to have a flag whose value more closely matches enable/disable (so 1 
 for enable, etc.) and default it to on, than to have a 'disable' tunable.
 

The decision was made to make it easy to support MSIX in future.
If controller supports both MSIX and MSI, the suggested scheme may
confuse users but I don't have strong opinion on that so will
follow your suggestion.

Thank you.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r225405 - head/sys/dev/ixgbe

2011-09-05 Thread YongHyeon PYUN
On Mon, Sep 05, 2011 at 05:54:19PM +, Qing Li wrote:
 Author: qingli
 Date: Mon Sep  5 17:54:19 2011
 New Revision: 225405
 URL: http://svn.freebsd.org/changeset/base/225405
 
 Log:
   The maximum read size of incoming packets is done in 1024-byte increments.
   The current code was rounding down the maximum frame size instead of
   routing up, resulting in a read size of 1024 bytes, in the non-jumbo
   frame case, and splitting the packets across multiple mbufs.
   

I guess the minimum allowed value of rx_mbuf_sz is 2048 such that
old code will still produce 2(i.e. 2KB).  Do you use non-standard
rx_mbuf_sz which is not multiple of 1024?

   Consequently the above problem exposed another issue, which is when
   packets were splitted across multiple mbufs, and all of the mbufs in the
   chain have the M_PKTHDR flag set.
   
   Submitted by:   original patch by Ray Ruvinskiy at BlueCoat dot com
   Reviewed by:jfv, kmacy, rwatson
   Approved by:re (rwatson)
   MFC after:  5 days
 
 Modified:
   head/sys/dev/ixgbe/ixgbe.c
 
 Modified: head/sys/dev/ixgbe/ixgbe.c
 ==
 --- head/sys/dev/ixgbe/ixgbe.cMon Sep  5 17:45:24 2011
 (r225404)
 +++ head/sys/dev/ixgbe/ixgbe.cMon Sep  5 17:54:19 2011
 (r225405)
 @@ -3849,6 +3849,8 @@ fail:
   **/
  #define IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT 2
  
 +#define BSIZEPKT_ROUNDUP ((1IXGBE_SRRCTL_BSIZEPKT_SHIFT)-1)
 + 
  static void
  ixgbe_initialize_receive_units(struct adapter *adapter)
  {
 @@ -3882,7 +3884,7 @@ ixgbe_initialize_receive_units(struct ad
   hlreg = ~IXGBE_HLREG0_JUMBOEN;
   IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg);
  
 - bufsz = adapter-rx_mbuf_sz   IXGBE_SRRCTL_BSIZEPKT_SHIFT;
 + bufsz = (adapter-rx_mbuf_sz + BSIZEPKT_ROUNDUP)  
 IXGBE_SRRCTL_BSIZEPKT_SHIFT;
  
   for (int i = 0; i  adapter-num_queues; i++, rxr++) {
   u64 rdba = rxr-rxdma.dma_paddr;
 @@ -4300,9 +4302,10 @@ ixgbe_rxeof(struct ix_queue *que, int co
   sendmp = rbuf-fmp;
   rbuf-m_pack = rbuf-fmp = NULL;
  
 - if (sendmp != NULL) /* secondary frag */
 + if (sendmp != NULL) {  /* secondary frag */
 + mp-m_flags = ~M_PKTHDR;
   sendmp-m_pkthdr.len += mp-m_len;
 - else {
 + } else {
   /* first desc of a non-ps chain */
   sendmp = mp;
   sendmp-m_flags |= M_PKTHDR;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r223648 - head/sys/dev/gem

2011-07-07 Thread YongHyeon PYUN
On Thu, Jul 07, 2011 at 11:26:08AM -0500, Nathan Whitehorn wrote:
 On 07/07/11 11:07, Marius Strobl wrote:
 On Thu, Jul 07, 2011 at 09:34:39AM -0500, Nathan Whitehorn wrote:
 This breaks one of my gem devices (chip=0x0021106b) when the controller
 is initialized from FreeBSD (netbooting works fine, and the controller
 stays working after that), with cannot disable RX MAC or cannot
 disable RX MAC or hash filter messages.
 Are you positive that it's exactly this revision which breaks things?
 Actually there was a report from Justin Hibbits that r222135 causing
 a GMAC to no longer work with similar symptoms, which still is unsolved
 AFAICT. If it's really r223648 which is causing problems in your case
 please try to figure out which part is causing that as none of the
 changes exactly stands out as disruptive, probably best by reverting
 gem_reset_rx() and gem_setladrf() one by one.
 
 
 You're right. r223648 just added the printf(), r222135 is where it began 
 not working.

Yes, I know that. I still have no clue and can't explain how
r222135 can break gem(4). Also note, the cannot disable RX MAC
was there before r222135.
One vague guess I have at this moment is that firmware may have put
controller into some deep power save state so the first time
gem_init() is called it may not have fully initialized the
controller. I sent a patch to Justin to check that and waiting for
his reply.

 -Nathan
Index: sys/dev/gem/if_gem.c
===
--- sys/dev/gem/if_gem.c	(revision 223682)
+++ sys/dev/gem/if_gem.c	(working copy)
@@ -328,6 +328,9 @@
 			phy = MII_PHY_ANY;
 			break;
 		}
+		if (GEM_IS_APPLE(sc))
+			GEM_BANK1_WRITE_4(sc, GEM_MAC_XIF_CONFIG,
+			GEM_MAC_XIF_TX_MII_ENA);
 		error = mii_attach(sc-sc_dev, sc-sc_miibus, ifp,
 		gem_mediachange, gem_mediastatus, BMSR_DEFCAPMASK, phy,
 		MII_OFFSET_ANY, MIIF_DOPAUSE);
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r220736 - head/sbin/natd

2011-04-18 Thread YongHyeon PYUN
On Mon, Apr 18, 2011 at 02:44:35PM -0700, Maxim Sobolev wrote:
 On 4/18/2011 11:13 AM, Gleb Smirnoff wrote:
 This looks like a hack and better place for this hack would be shell
 scripts rather than nat daemon.
 
 Well, I am not sure how would you apply shell script in such case. The 
 problem with the original code is that natd just silently exits, leaving 
 machine without a network connection. For some reason this problem 
 started after upgrade from 7.4 to 8.2, perhaps there is some changes in 
 the dhclient which allows it to run is parallel with other start-up 
 activity.
 

SYNCDHCP may restore old behavior of dhclient.

 And I don't see any problem with natd waiting indefinitely on the 
 interface to acquire IP address, it's no better and no worse than the 
 current behavior when the natd simply bails out.
 
 -Maxim
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org