<div dir="ltr">Hi Salvatore,<div><br></div><div>Thanks for the comments, responses inline:</div><div><br></div><div><span style="font-size:12.8000001907349px">> - several IPv4 options have variable length, e.g., LSRR, timestamp and others; so you need a variable length field into the option_A header type and you need a component/blackbox/whatever that can take another header field as parameter:</span><br style="font-size:12.8000001907349px"></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">Both of the option headers in this example were intended to be fixed length options. Indeed, variable-legnth options would need a star field. As defined, the counter's increment method can already take a header field as its argument. </span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">> </span><span style="font-size:12.8000001907349px">(why aren't option_A and option_B arrays too?)</span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">They could be - it's the difference between allowing those options to be repeated in the header or not. It's probably important to clarify in the spec what happens when one extracts to an already-extracted instance (I would lean towards an exception being triggered).</span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">> </span><span style="font-size:12.8000001907349px">- as regards the struct ipv4_t, I think of it as an ordered memory area, so if you define it like the following:</span></div><div><br></div>While the struct's members are ordered, I would not assume that this order reflects how data is actually deparsed to the wire - the spec currently describes the wire ordering as being inferred from the parse graph.<div><br></div><div>That being said, you still bring up a good point: if the deparsing mechanism of the language works by determining a fixed order of header instances (either through the struct definition or through the parse graph), it's difficult to preserve the order of header options like this. We've been working through a few different approaches to deal with this, and one possibility is to define a union-of-headers construct and allow arrays of these unions. A union containing every possible header option is defined, and then an array of these unions is parsed in the main option processing loop. It's a more advanced case of TLV parsing that we didn't want to address in our initial proposal, and is only an issue depending on how the deparsing process is defined.</div><div><br></div><div>~Leo</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 10, 2015 at 6:23 AM, Salvatore Signorello <span dir="ltr"><<a href="mailto:salvatore.signorello@uni.lu" target="_blank">salvatore.signorello@uni.lu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    Hi all,<span class=""><br>
    <br>
    <br>
    <div>On 08/07/2015 02:39 AM, Changhoon Kim
      wrote:<br>
    </div>
    <blockquote type="cite">[C] Barefoot proposes<span style="font-size:12.8000001907349px"> a blackbox that lets the
        parser keep such state, and says this is enough to handle common
        headers like IPv4, Geneve, and TCP.</span></blockquote>
    <br></span>
    follow few things I've noticed skimming the ipv4_options_parser.p4
    code. They're probably all known to the Barefoot folks, if so,
    please just ignore them.<br>
    <br>
    Best regards<br>
    <br>
    <br>
    The idea of the counter to parse recurrent similar fields sounds
    great, but as showed into the code sample doesn't parse the IPv4
    options for the following reasons:<br>
    <br>
    - several IPv4 options have variable length, e.g., LSRR, timestamp
    and others; so you need a variable length field into the option_A
    header type and you need a component/blackbox/whatever that can take
    another header field as parameter:<br>
    <br>
    header_type ipv4_option_A {<br>
        fields {<br>
            // Boilerplate<br>
            bit<8> type;<br>
            bit<8> len; (you cannot use 'length' as field_name,
    because this is a reserved token id)<br>
    <br>
            // Option A fields<br>
            bit<*> foo;<br>
        }<br>
        length : len; // this is the header length in bytes <br>
        max_length : 40;<br>
    }<br>
    <br>
    <br>
    parser parse_option_a {<br>
            extract(ipv4.option_A);<br>
            ipv4_byte_counter.increment(-ipv4.option_A.len);<br>
            return options_loop;<br>
    }<br>
    <br>
    <br>
    - as regards the struct ipv4_t, I think of it as an ordered memory
    area, so if you define it like the following:<br>
    <br>
    struct_type ipv4_t {<br>
        header ipv4_base_t base;<br>
        header ipv4_option_A option_A; (why aren't option_A and option_B
    arrays too?)<br>
        header ipv4_option_B option_B;<br>
        header ipv4_option_padding padding[4]; (why 4? at max you need 3
    to fill a 4 bytes word when a 1B option is present)<br>
    }<br>
    <br>
    it does mean that you can have option_A then option_B then
    eventually some padding. Indeed this is not true, as for example the
    "No operation" option (RFC 791) is meant to align the beginning of
    the next option to a 32bit boundary. So you might (I've not run any
    code to double-check it) have:<br>
    <br>
    option_A[0] + option_B[0] + option_A[1] + option_B[0] (this time is
    an 'end of options list', in RFC 791 too ) + padding[0]<span class="HOEnZb"><font color="#888888"><br>
    <br>
    <br>
    <br>
    <br>
    <pre cols="72">-- 
Salvatore Signorello
PhD student @ SecanLab

Interdisciplinary Centre for Security, Reliability and Trust
SnT, University of Luxembourg
<a href="http://wwwen.uni.lu/snt/people/salvatore_signorello" target="_blank">http://wwwen.uni.lu/snt/people/salvatore_signorello</a></pre>
  </font></span></div>

<br>_______________________________________________<br>
P4-design mailing list<br>
<a href="mailto:P4-design@p4.org">P4-design@p4.org</a><br>
Listinfo - <a href="http://mail.p4.org/mailman/listinfo/p4-design_p4.org" rel="noreferrer" target="_blank">http://mail.p4.org/mailman/listinfo/p4-design_p4.org</a><br>
<br>
<br></blockquote></div><br></div>