On 15.12.2015 23:20, Joe Hershberger wrote:
On Fri, Dec 11, 2015 at 6:03 AM, Michal Simek michal.simek@xilinx.com wrote:
When IP is configured with pong buffers, IP is receiving packets to ping and then to pong buffer and than ping again. Origin logic in the driver remains there that when ping buffer is
Origin -> Original
fixed
free, pong buffer is checked too and return if both are free.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Do you know macros which I could use for addressing certain fields in ethernet and IP packet? The code is there because copying huge amount of data is causing performance penalty.
So you're saying the IP will not tell you the length of the frame received? You have to pull it out of the packet data?
Unfortunately yes. Linux driver uses the same logic. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers...
drivers/net/xilinx_emaclite.c | 90 ++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 39 deletions(-)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c index e97ce2ce31f3..b5ff4f099251 100644 --- a/drivers/net/xilinx_emaclite.c +++ b/drivers/net/xilinx_emaclite.c @@ -22,11 +22,6 @@
#define ENET_ADDR_LENGTH 6
Use ARP_HLEN from include/net.h
-/* EmacLite constants */ -#define XEL_BUFFER_OFFSET 0x0800 /* Next buffer's offset */ -#define XEL_RSR_OFFSET 0x17FC /* Rx status */ -#define XEL_RXBUFF_OFFSET 0x1000 /* Receive Buffer */
/* Xmit complete */ #define XEL_TSR_XMIT_BUSY_MASK 0x00000001UL /* Xmit interrupt enable bit */ @@ -86,7 +81,7 @@ struct emaclite_regs { };
struct xemaclite {
u32 nextrxbuffertouse; /* Next RX buffer to read from */
bool nextrxbuffertouse; /* Next RX buffer to read from */When using a boolean for this sort of thing it is good to give it a more clear name, such as "use_rx_pong_buffer_next".
done.
u32 txpp; /* TX ping pong buffer */ u32 rxpp; /* RX ping pong buffer */ int phyaddr;@@ -455,45 +450,63 @@ static int emaclite_recv(struct eth_device *dev) { u32 length; u32 reg;
u32 baseaddress;
u32 *addr, *ack; struct xemaclite *emaclite = dev->priv;
baseaddress = dev->iobase + emaclite->nextrxbuffertouse;reg = in_be32 (baseaddress + XEL_RSR_OFFSET);debug("Testing data at address 0x%x\n", baseaddress);if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {if (emaclite->rxpp)emaclite->nextrxbuffertouse ^= XEL_BUFFER_OFFSET;
struct emaclite_regs *regs = emaclite->regs;u32 attempt = 0;+try_again:
if (!emaclite->nextrxbuffertouse) {reg = in_be32(®s->rx_ping_rsr);debug("Testing data at rx_ping\n");if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {debug("Data found in rx_ping buffer\n");addr = ®s->rx_ping;ack = ®s->rx_ping_rsr;} else {debug("Data not found in rx_ping buffer\n");/* Pong buffer is not available - return immediatelly */immediatelly -> immediately
fixed.
if (!emaclite->rxpp)return -1;/* Try pong buffer if this is first attempt */if (attempt++)return -1;emaclite->nextrxbuffertouse =!emaclite->nextrxbuffertouse;goto try_again;} } else {
if (!emaclite->rxpp) {debug("No data was available - address 0x%x\n",baseaddress);return 0;
reg = in_be32(®s->rx_pong_rsr);debug("Testing data at rx_pong\n");if ((reg & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {debug("Data found in rx_pong buffer\n");addr = ®s->rx_pong;ack = ®s->rx_pong_rsr; } else {
baseaddress ^= XEL_BUFFER_OFFSET;reg = in_be32 (baseaddress + XEL_RSR_OFFSET);if ((reg & XEL_RSR_RECV_DONE_MASK) !=XEL_RSR_RECV_DONE_MASK) {debug("No data was available - address 0x%x\n",baseaddress);return 0;}
debug("Data not found in rx_pong buffer\n");/* Try ping buffer if this is first attempt */if (attempt++)return -1;emaclite->nextrxbuffertouse =!emaclite->nextrxbuffertouse;goto try_again; } }+#define ETH_TYPE_OFFSET 3
This is offset in count of 32-bit words.
yep
+#define IP_LENGTH_OFFSET 4 /* Get the length of the frame that arrived */
switch(((ntohl(in_be32 (baseaddress + XEL_RXBUFF_OFFSET + 0xC))) &0xFFFF0000 ) >> 16) {
switch (((ntohl(in_be32(addr + ETH_TYPE_OFFSET))) & 0xFFFF0000) >> 16) {Why not read 16 bits? Not possible in hardware?
There are a lot of versions of this IP. It is BE/LE driver and there could be HW issues too. But look below.
case 0x806:Use PROT_ARP from include/net.h
done.
length = 42 + 20; /* FIXME size of ARP */Not sure what this is trying to include, but that seems like it is too big.
From net.h: #define ETHER_HDR_SIZE (sizeof(struct ethernet_hdr)) which equals 14 bytes. #define ARP_HDR_SIZE (8+20)
So it should add up to 14 + 8 + 20 = 42;
I would use:
length = ETHER_HDR_SIZE + ARP_HDR_SIZE;
Based on http://image.slidesharecdn.com/arp-140623072259-phpapp01/95/arp-6-638.jpg?cb...
+ 10 padding and +4 crc. I see that tsec driver does something with crc but maybe crc are completely ignored by U-Boot
On the other hand linux is using just ETH_FCS_LEN (4). I have added +4 for now.
debug("ARP Packet\n");
debug("ARP Packet %x\n", length); break; case 0x800:Use PROT_IP from include/net.h
fixed.
length = 14 + 14 +
(((ntohl(in_be32 (baseaddress + XEL_RXBUFF_OFFSET +0x10))) & 0xFFFF0000) >> 16);/* FIXME size of IP packet */debug ("IP Packet\n");
(((ntohl(in_be32(addr + IP_LENGTH_OFFSET))) &0xFFFF0000) >> 16);Why not read 16 bits?
look below.
If this is reading the total_length field, it seems that it will not handle packet fragments well.
Isn't it fragmentation done on one layer up? IP packet header is there all the time.
debug("IP Packet %x\n", length); break;Can you afford to read the first to read the first 5 words of the packet into a buffer and use the structs in net.h to overlay the buffer to access the data? That would be a lot cleaner to look at.
I have tried it and it is working fine. I am not reading just 5 words but I think it is better to read the full align ARP packet size and then do another read for the rest. Please look at v2 with this change I think you will like it more than this one.
You also need to somehow track fragmented packets. Not sure how to do it universally. You sure there's no way to get the frame size from the hardware? That seems very limiting.
I haven't seen any problem like this and I expect none has played with it too.
Thanks, Michal