Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

agent: listen over a serial interface #5

Open
2 of 4 tasks
GridexX opened this issue Feb 8, 2023 · 15 comments · May be fixed by #12
Open
2 of 4 tasks

agent: listen over a serial interface #5

GridexX opened this issue Feb 8, 2023 · 15 comments · May be fixed by #12

Comments

@GridexX
Copy link
Contributor

GridexX commented Feb 8, 2023

This is one of the first issue related to the agent.

For now we have defined the following tasks:

  • Define the spec that will be used for the communication over the serial port. The SLIP protocol seems to be a good starting point.
  • Define what will be send through the serial port.
  • Implement the interface in RUST
  • Tests that the behavior is working as expected

This is still an early proposal of the tasks to do. We need further exploring before truly having a grasp on what needs to be done. Please comment and discuss with us anything we missed or misunderstood, or if this does not look like a logical division of tasks.

@alexis-ascoz
Copy link

alexis-ascoz commented Feb 17, 2023

We have define the following json format to communicate bewteen the vmm.

{
  "files": [
    {
      "filename": "src/index.js",
      "content": "console.log('Hello World!');"
    }
  ],
  "script": ["node src/index.js"]
}

@iverly
Copy link
Contributor

iverly commented Feb 24, 2023

We have define the following json format to communicate bewteen the vmm.

{
  "files": [
    {
      "filename": "src/index.js",
      "content": "console.log('Hello World!');"
    }
  ],
  "script": ["node src/index.js"]
}

This JSON is incorrect. You should implement the output directive as we defined on the lambdo configuration file.
PS: Rename script to steps pls

@alexis-ascoz
Copy link

alexis-ascoz commented Mar 1, 2023

We have define the following json format to communicate bewteen the vmm.

{
  "files": [
    {
      "filename": "src/index.js",
      "content": "console.log('Hello World!');"
    }
  ],
  "script": ["node src/index.js"]
}

This JSON is incorrect. You should implement the output directive as we defined on the lambdo configuration file. PS: Rename script to steps pls

Sure, the payload should like the following :

{
  "files": [
    {
      "filename": "src/index.js",
      "content": "console.log('Hello World!');"
    }
  ],
  "steps": [
    {
      "command": "node src/index.js",
      "enableOutput": true
    }
  ]
}

For a step, there is two new fields :

  • enabled: This field indicates if the command need to be listened

The result can be use or not for the PolyCode validator

@GridexX
Copy link
Contributor Author

GridexX commented Mar 1, 2023

We have defined with our team the protocol that shound be used to speak with the agent:

Each message should be delimited at the beginning and the end of the message with the character 0x1c:

image

Between these delimliters, payload will be send through JSON.

3 types of message existed which are specified in the type field.

statusMessage

When the AGENT starts, it send a statusMessage with the following payload :

For the ready message sending at startup

{
"type": "statusMessage",
"status" : "OK"
}

codeRequest

{
  "type": "codeRequest",
  "data": {
    "files": [
      {
        "filename": "src/index.js",
        "content": "console.log('Hello World!');"
      }
    ],
    "steps": [
      {
        "command": "node src/index.js",
        "enableOutput": true
      }
    ]
  }
}

codeExecution

Here is the payload that is transmitted after a codeExecution. For each commands send, we return the whole result. We won't maniuplate the stdout or stderr stream, but we wouldn't send back the result of those stream if enableOutput = false :

With enableOutput = false

{
  "type": "codeExecution",
  "data": {
    "steps": [
    {
      "command": "node src/index.js",
      "result": 0,
      "stdout": "",
      "stderr": "",
      "enableOutput": false
      }
    ]
  }
}

With enableOutput = true

{
  "type": "codeExecution",
  "data": {
    "steps": [
    {
      "command": "node src/index.js",
      "result": 0,
      "stdout": "Hello World!",
      "stderr": "",
      "enableOutput": true
      }
    ]
  }
}

📓 The enableOutput field is only used to avoid useless network communication.

@alexis-langlet
Copy link

I understand that enableOutput is used to avoid useless network communication but shouldn't we always return the stderr ?
It will allow to trace and debug more easily in case of a problem.

@RedbeanGit
Copy link
Contributor

Why chosing 0x1c as delimiter? You're not afraid that a malicious person send a 0x1c character in his program?

@sameo
Copy link
Contributor

sameo commented Mar 5, 2023

A few comments:

  • If 0x1c is your delimiter, you need to parse the payload and escape any 0x1c with an escape character. You also need to escape your escape character, etc. This is roughly what SLIP does, hence my advice. See https://en.wikipedia.org/wiki/Serial_Line_Internet_Protocol
  • You want stderr to be returned unconditionally
  • You probably want to add a UUID to your workload sent to the agent, and have the agent sent that UUID back when the function has been executed. That will allow you to have one host listener for multiple VMs.
  • You want to identify your messages through a type and a Code. type would be either Request, Response or Status. And Code could be Check, Execute, Kill, Abort, etc. A regular, flawless flow would look like that:
Agent  --- Type: Status, Code: Ok ---> Host
Agent <--- Type: Request, Code: Run --- Host
Agent  --- Type: Response, Code: Run ---> Host

@alexis-ascoz
Copy link

Hello @sameo, we have read your remarks
We agree on the three first point and will modify our code accordingly

Concercing the last point, we didn't understand why we should have a Code field. Could you please elaborate on this fields' purpose ?
We feel like you want us to run code asynchronously, meaning that we can abort it with signals from the host.
As we agreed, VM should only launch one code execution. So when the host wants to stop it, he has to stop directly the VM.

@sameo
Copy link
Contributor

sameo commented Mar 19, 2023

Sorry for the delayed reply @Alexis-Bernard

Concercing the last point, we didn't understand why we should have a Code field. Could you please elaborate on this fields' purpose ? We feel like you want us to run code asynchronously, meaning that we can abort it with signals from the host. As we agreed, VM should only launch one code execution. So when the host wants to stop it, he has to stop directly the VM.

You are right, this is the current scope, and cancelling a workload could be done that way. However, a frame Code field, combined with your Type one, will give you a lot of flexibility and allow for future protocol extensions at a cost of basically one byte per frame. You can ignore it for now, or only support Run, and that's fine. But if you want to ever extend the number of operations that your protocol supports, having this field there will let you do so without breaking backward compatibility.

@GridexX
Copy link
Contributor Author

GridexX commented Mar 21, 2023

@Maxtho8 We change a bit of fields, please think to update your code accordingly 😄

Okay, here are the final objects defining the communication protocol.

We will use the SLIP protocol for transferring messages as suggested.

⚠️ We will keep lower case letter for all the payload

Status of the agent

The first message send by the agent to the host will be the following JSON payload :

{
  "type": "status",
  "code": "ready"
}

Code request

This message should be send by the Host to the Agent and request a code execution. An UUID should be provided to identity the workload through the id field.

{
  "type": "request",
  "code": "run",
  "data": {
    "id": "4bf68974-c315-4c41-aee2-3dc2920e76e9",
    "files": [
      {
        "filename": "src/index.js",
        "content": "console.log('Hello World!');"
      }
    ],
    "steps": [
      {
        "command": "node src/index.js",
        "enableOutput": true
      }
    ]
  }
}

Enable output will only hide the stdout field

Code response

{
  "type": "response",
  "code": "run",
  "data": {
    "id": "4bf68974-c315-4c41-aee2-3dc2920e76e9",
    "steps": [
      {
        "command": "node src/index.js",
        "exitCode": 0,
        "stdout": "",
        "stderr": ""
        }
      ]
   } 
}

For each command, the following field will be returned:

  • The return code through the field result. If != 0, the execution failed
  • The stdout field will always be present but contains an empty string in case enableOutput = false
  • Stderr contains the stderr stream

@sameo
Copy link
Contributor

sameo commented Mar 21, 2023

@GridexX Good progress. One remaining question on the response frame:

{
  "type": "response",
  "code": "run",
  "data": {
    "id": "4bf68974-c315-4c41-aee2-3dc2920e76e9",
    "steps": [
      {
        "command": "node src/index.js",
        "result": 0,
        "stdout": "",
        "stderr": "",
        "enableOutput": false
        }
      ]
   } 
}

Are you planning to support providing a result and output for each steps?

@GridexX
Copy link
Contributor Author

GridexX commented Mar 21, 2023

@sameo Thanks for the quick reply !

Yes, we plan to support providing a result and output for each step !

@Maxtho8
Copy link

Maxtho8 commented Mar 21, 2023

Hello,

There was a communication problem because with the other teams we had decided not to use SLIP and delimiter.

By default the system is waiting for a frame in this form :

RDY-Tram

The first tram is RDY with a message size of 0 to tell to VMM that agent is ready and we can send code. There is 3 status code RDY , EXE and RES

After VMM send the following frame :

TRAM-exe

When the agent receive an EXE frame , he execute the code passed in the JSON message

To finish, the VMM is waiting for the RES frame

We had problems with the slip protocol what do you think of this alternative?

@Maxtho8
Copy link

Maxtho8 commented Mar 22, 2023

Hello,

After discussing with the agent team, we realized that we both had problems with SLIP lib.
Instead of implementing SLIP ourself , we decide to use the protocol that I introduced just before but without status code.
We send the messages size (number of chars) in the first 8 chars and the JSON in the same format that was previously decided

@sameo
Copy link
Contributor

sameo commented Mar 26, 2023

Hello,

There was a communication problem because with the other teams we had decided not to use SLIP and delimiter.

By default the system is waiting for a frame in this form :

RDY-Tram

The first tram is RDY with a message size of 0 to tell to VMM that agent is ready and we can send code. There is 3 status code RDY , EXE and RES

EXE and RES are typically not considered to be status frames, but rather respectively a request and a response.

After VMM send the following frame :

TRAM-exe

A few comments here:

  • You're not making any difference between host to guest (Request), guest to host (Response or Status). Are guests allowed to send requests as well? By defining your frames as either a request, a response, or an async status, you're clearly expressing the asymmetry of the protocol (Host is the initiator, guest is the responder and is allowed to send asynchronous status).
  • Using 3 bytes for a command/response or status is quite inefficient.
  • Is the message size a decimal number, expressed as a string??
  • There's only one valid reason I can think about for using strings as payload header: Making them easily human readable. If you really want to do that, then just use a 16-bit header for your payload length and have it immediatly followed by a JSON payload that would also contain your frame type:
┌───────────────────┬──────────────────────────────────────────────────────┐
│   Frame Length    │                        JSON                          │
│      16 bits      │                       Payload                        │
└───────────────────┴──────────────────────────────────────────────────────┘

When the agent receive an EXE frame , he execute the code passed in the JSON message

To finish, the VMM is waiting for the RES frame

We had problems with the slip protocol what do you think of this alternative?

Does this protocol define a frame delimiter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants