Minor tweaks to OpenOS/etc/rc.d/minitel.lua to improve performance

[Jajasek]

1. When a server is repeating a packet, the destination is cached in rcache and rcache[dest][1] is the same modem the packet arrived through, the packet should not be repeated. This could be solved by adding a condition in the function sendPacket (on line 119).
2. The comment on line 37 suggests the time in rcache should be updated when a new packet from that computer is received, but it is not implemented. The function processPacket writes to rcache only if the computer is not cached. The condition on line 189 should be removed, the line
   rcache[sender] = {localModem,from,computer.uptime()+cfg.rctime}
   should be executed for all packets. Therefore when 2 computers periodically communicate, the servers between them never forgot the route. Also, it should be executed before the ack packet is sent. I therefore suggest to move it before the line 172.
3. Cosmetic issue: the packets contain the vPort entry, but it is not documented in the comment on line 10.

[Jajasek]

I have done some more playtesting and revealed another 2, much more severe, mistakes.
4. In file OpenOS/usr/lib/minitel.lua, function net.rsend() returns true if any ack packet is seen, not only the one with rpid == pid.
5. The broadcast messages are totally broken. I wonder how could this error persist here for so long time. When the function processPacket() encounters a broadcast message, it pushes net_broadcast signal, but the arrangement of the conditionals prevent it from routing the packet. I fixed it and this is my solution (it fixes even problem 2 mentioned above):

local function processPacket(_,localModem,from,pport,_,packetID,packetType,dest,sender,vPort,data,...)
  pruneCache()
  if pport == cfg.port or pport == 0 then -- for linked cards
    dprint(cfg.port,vPort,packetType,dest)
    if checkPCache(packetID) then return end
    dprint("rcache: "..sender..":", localModem,from,computer.uptime())
    rcache[sender] = {localModem,from,computer.uptime()+cfg.rctime}
    if dest:sub(1,1) == "~" then -- broadcasts start with ~
      computer.pushSignal("net_broadcast",sender,vPort,data)
    end
    if dest == hostname then
      if packetType == 1 then
        sendPacket(genPacketID(),2,sender,hostname,vPort,packetID)
      end
      if packetType == 2 then
        dprint("Dropping "..data.." from queue")
        pqueue[data] = nil
        computer.pushSignal("net_ack",data)
      end
      if packetType ~= 2 then
        computer.pushSignal("net_msg",sender,vPort,data)
      end
    elseif cfg.route then -- repeat packets if route is enabled
      sendPacket(packetID,packetType,dest,sender,vPort,data,localModem)
    end
    if not pcache[packetID] then -- add the packet ID to the pcache
      pcache[packetID] = computer.uptime()+cfg.pctime
    end
  end
end

[Leonard2]

Thanks a lot for the fix!

[Kurtoid]

Can you add this as a pull request?

[RedstoneParkour]

late reply, but issue number 5 is intentional:

Broadcast packets MUST NOT be repeated, in order to keep them within the same layer 2 network, unless special precautions are taken. If a packet is multicast and you don't support multicast, then you SHOULD NOT repeat the packet.

[Jajasek]

@kcinnaJlol Thanks for the reply, even though I have since abandoned my project. Nevertheless, could you please elaborate, for the sake of my education? I am not sure I fully understand the reason. And if you are quoting from some source, could please you post a link?