Ethernet MAC component 2.3.3 - Configuration Issue or Bug?

[Jack]

@redeye:
2.3.0rc0 still works fine! The problem starts with 2.3.1rc0 and is very likely located in mii_master.xc

2.3.1rc0 contains the fix that prevents invalid IFG and therfor quite helpful. Unfortunately by this fix we get the misbehaviour with alternate blocking/no blocking of outgoing frames, if no traffic was sent for more then 21s.

Any idea how to fix this?

[Redeye]

@Jack I have to admit I don't really understand what/why they've done in that IFG fix, but it clearly doesn't work. For now I can live without the IFG fix as it doesn't appear to be causing problems, though I would like to fix it if possible.

I guess my first step would be to compare against the equivalent code in the current v3.x ethernet module, though I'm not sure whether the changes between v2.x and v3.x are so big that this part of the code will be incomparable.

[peter]

Thanks for all your feedback. I can confirm this is an issue with the IFG implementation. The change was made to ensure that the inter-frame gap was being respected as the previous implementation did send out packets with a sub 960ns IFGs.

However, the code did have an underlying assumption that there had to be at least one packet every 21.47s, and if there wasn't then the timing was broken and the packet transmission ends up pausing as the hardware timer had wrapped. Clearly this is not good.

I have just done a fix for this on lib_ethernet (the repo XMOS is currently maintaining): https://github.com/xmos/lib_ethernet/pull/3

This uses a bit of assembly to set up the timer straight away and can therefore never wrap, regardless of how long it is before the user comes to send another packet.

And for those using sc_ethenet, I've created a pull request that you can use: https://github.com/xcore/sc_ethernet/pull/61

However, that one is not tested other than that it compiles.

[Jack]

@Peter:
Thanks a lot for the fix! It seems, that the modified version of mii_master.xc requires 2 additional timers. Unfortunatly we use ethernet_server_full_two_port (sc_ethernet) that already requires 9 timers using the original mii_master.xc. The fix therfore exceeds the maximum resources (10 timers per tile) by 1 timer. Is there any chance to have a fix of mii_master.xc saving one timer?

[peter]

Hmm, there is only one way I can think to do that, but it means moving the IFG enforcement to before the credit timer is updated. This will reduce the peak TX rate, but will not require the extra timer. Try applying the following patch on top of the pull request code (save the file as 'patch') and then do:

patch -p1 < patch

in sc_ethernet...

Code: Select all

diff --git a/module_ethernet/src/full/mii_master.xc b/module_ethernet/src/full/mii_master.xc
index 54d96d3..0eabd74 100644
--- a/module_ethernet/src/full/mii_master.xc
+++ b/module_ethernet/src/full/mii_master.xc
@@ -38,7 +38,7 @@
 // that reads the timer is the next instruction after the out at the
 // end of the packet and the timer wait is an instruction before the
 // out of the pre-amble
-#define ETHERNET_IFS_AS_REF_CLOCK_COUNT  (96 + 96 - 10)
+#define ETHERNET_IFS_AS_REF_CLOCK_COUNT  (96 + 96 - 20)
 
 // Receive timing constraints
 #if ETHERNET_ENABLE_FULL_TIMINGS
@@ -409,7 +409,7 @@ int g_mii_idle_slope[NUM_ETHERNET_PORTS];
 
 
 // Do the real-time pin wiggling for a single packet
-unsigned mii_transmit_packet(unsigned buf, out buffered port:32 p_mii_txd, hwtimer_t tmr, unsigned ifg_time)
+unsigned mii_transmit_packet(unsigned buf, out buffered port:32 p_mii_txd, hwtimer_t tmr)
 {
     register const unsigned poly = 0xEDB88320;
     unsigned int crc = 0;
@@ -426,11 +426,6 @@ unsigned mii_transmit_packet(unsigned buf, out buffered port:32 p_mii_txd, hwtim
     dptr = mii_packet_get_data_ptr(buf);
     wrap_ptr = mii_packet_get_wrap_ptr(buf);
 
-    // Check that we are out of the inter-frame gap
-    asm volatile ("in %0, res[%1]"
-                  : "=r" (ifg_time)
-                  : "r" (tmr));
-
 #pragma xta endpoint "mii_tx_sof"
     p_mii_txd <: 0x55555555;
     p_mii_txd <: 0xD5555555;
@@ -517,15 +512,12 @@ void mii_tx_pins(
 #if (ETHERNET_TX_HP_QUEUE) && (ETHERNET_TRAFFIC_SHAPER)
     int credit = 0;
     int credit_time;
-    // Need one timer to be able to read at any time for the shaper
-    timer credit_tmr;
 #endif
-    // And a second timer to be enforcing the IFG gap
-    hwtimer_t tmr;
+    timer tmr;
     unsigned ifg_time;
 
 #if (ETHERNET_TX_HP_QUEUE) && (ETHERNET_TRAFFIC_SHAPER)
-    credit_tmr :> credit_time;
+    tmr :> credit_time;
 #endif
     tmr :> ifg_time;
     while (1) {
@@ -563,9 +555,14 @@ void mii_tx_pins(
         }
 #endif
 
+    // Check that we are out of the inter-frame gap
+    asm volatile ("in %0, res[%1]"
+                  : "=r" (ifg_time)
+                  : "r" (tmr));
+
 #if (ETHERNET_TRAFFIC_SHAPER)
         prev_credit_time = credit_time;
-        credit_tmr :> credit_time;
+        tmr :> credit_time;
 
         elapsed = credit_time - prev_credit_time;
         credit += elapsed * idle_slope;
@@ -620,7 +617,7 @@ void mii_tx_pins(
         }
 
 #pragma xta endpoint "mii_tx_start"
-        unsigned prev_eof_time = mii_transmit_packet(buf, p_mii_txd, tmr, ifg_time);
+        unsigned prev_eof_time = mii_transmit_packet(buf, p_mii_txd, tmr);
 
         ifg_time = prev_eof_time + ETHERNET_IFS_AS_REF_CLOCK_COUNT;
         ifg_time += (mii_packet_get_length(buf) & 0x3) * 8;

[Jack]

Thanks again for the support, Peter.
We patched the file but unfortunately get an error message when compiling: incompatible type for argument 3 of `mii_transmit_packet' line 622
Find attached the resulting file "mii_master.xc" after patching

mii_master.xc

[peter]

Sorry, managed to get the timer type wrong in the patch. It should just be:

Code: Select all

unsigned mii_transmit_packet(unsigned buf, out buffered port:32 p_mii_txd, timer tmr)

Code: Select all

unsigned mii_transmit_packet(unsigned buf, out buffered port:32 p_mii_txd, hwtimer_t tmr)

[Jack]

Now it seems to work as required by our application. Thanks to all for the support!