CPqD / ofsoftswitch13

OpenFlow 1.3 switch.
http://cpqd.github.com/ofsoftswitch13
301 stars 193 forks source link

error setting mpls label values greater than 256 #267

Closed tresegiangi closed 6 years ago

tresegiangi commented 6 years ago

Hi all, I try to use the softswitch as mpls router and I obtain a bug when I try to push/pop mpls label greater than 256. The value setting in the openflow message is not correct (seems a trunk problem of 20 bit label). Just for example when I try the command: dpctl tcp:192.168.2.6:44501 flow-mod table=0,cmd=add in_port=1,eth_type=0x8847,mpls_label=1000000 apply:pop_mpls=0x8847,push_mpls=0x8847,set_field=mpls_label:1,set_field=mpls_bos:1,mpls_ttl=128,output=2

The output is: SENDING: flow_mod{table="0", cmd="add", cookie="0x0", mask="0x0", idle="0", hard="0", prio="32768", buf="none", port="any", group="any", flags="0x0", match=oxm{mpls_label="64", in_port="1", eth_type="0x8847"}, insts=[apply{acts=[mpls_pop{eth="0x8847"}, mpls_psh{eth="0x8847"}, set_field{field:mpls_label="1"}, set_field{field:mpls_bos="0"}, mpls_ttl{ttl="128"}, out{port="2"}]}]}

OK.

It seems that there is a trunk from 20 bit label value to 8 bits during the mpls label formatting. Have you encountered a similar error? Thanks

ederlf commented 6 years ago

The max value to be checked is clearly wrong. Likely to be a copy and paste problem. https://github.com/CPqD/ofsoftswitch13/blob/master/utilities/dpctl.c#L1398

tresegiangi commented 6 years ago

I check this part of code and the function seems correct (I try to print the "mpls_label" variable at the output of the function and the value is correct). The same error is obtained also when setting the mpls label in the action field (like push a label greater of 256). It seems that the problem is the copy/paste of the value in the "ofl_match_tlv" struct. Am I wrong?

ederlf commented 6 years ago

Indeed, I quick checked the max value and it looked like the max value was smaller, but you are right.

The problem is actually only in the printing of the mpls label https://github.com/CPqD/ofsoftswitch13/blob/a2bd7b01339f2c3cdb28cf416d4b176feea97b0c/oflib/ofl-structs-print.c#L392

Casting is

((uint32_t) *f->value)

But should be

*((uint32_t*) f->value)
ederlf commented 6 years ago

Commit 70fe1882a893cc901ed9e676fb1322d87f48899e fix that. Please try and let me know if the issue can be closed.

tresegiangi commented 6 years ago

Try the commit and it works. So the issue can be closed. Thank you for you support!!!