<div dir="ltr"><div dir="ltr"><div>Hi Isaac,</div><div><br></div><div>I have tried to avoid making the SimpleSwitch object global, as I may add support for running multiple instances in the same process in the future.</div><div><b>If you need to have access to the SimpleSwitch instance inside your extern, maybe you should consider not using the REGISTER_PRIMITIVE macro, but instead calling bm::ActionOpcodesMap::get_</b><b>instance()->register_primitive directly in the SimpleSwitch constructor. This should let you pass a pointer to SimpleSwitch to your primitive / extern constructor if you capture it in the lambda.</b></div><div>Whether primitives / externs should have access to the switch base class (SwitchWContexts) is up for debate. I believe that giving them access to the P4Objects instance is enough, but I could be convinced otherwise if I see a pretty compelling use case. What I do know is that implementing this change properly would be quite disruptive.</div><div><br></div><div>As for your final question, it is important to remember that simple_switch is meant specifically to run P4 programs written for the v1model architecture. Adding new externs mean that your P4 program is technically not a v1model program any more, and although it is relatively easy to extend simple_switch to support your new extern, your program is no longer considered portable across targets that claim v1model support. What that means is that we are probably not going to add new extern support to simple_switch, unless this extern is also added to v1model (unlikely). Maintaining your own bmv2 target is the right thing to do; it can be annoying to pull in the latest upstream changes but I don't expect simple_switch to change that much in the future (we will be focusing on the PSA implementation I hope).<br></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 4, 2018 at 12:40 PM Isaac Pedisich <<a href="mailto:iped@seas.upenn.edu" target="_blank">iped@seas.upenn.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi Antonin,</div><div><br></div><div>Thanks for your reply. Following up on this: </div><div>The
 modifications to simple switch that we have made to allow for new 
packet injection into arbitrary stages of the switch pipeline require 
access to the switch object from within an extern. To enable this 
functionality, we have modified the simple switch to provide a global, 
singleton reference so it can be accessed when necessary.</div><div><br></div><div>While this fits our needs, I'm aware the implementation is not ideal for many cases, and am interested to find out if, perhaps with a little more work, I can arrive at an implementation that can benefit the wider P4 BMv2 community. If you have any guidance on a
better way to implement the feature I could try to go about it.<br>
</div><div><br></div><div>In case it's not possible for me to develop a more acceptable patch within the time I can commit, I'd like to ask if anybody else on the list has advice on maintaining out-of-tree patches in our BMv2 fork moving<span class="m_5357458791756490545m_-8509286553622062463gmail-im"> forward, as the bmv2 repo undoubtedly undergoes changes.<br></span></div><div><br><span class="m_5357458791756490545m_-8509286553622062463gmail-im"></span></div><div>My
 question is: given few but significant changes to a behavioral model 
target, how have other teams maintained their code (which may not be 
suitable for inclusion in the behavioral-model repo) while keeping 
up-to-date with changes? Ultimately, we are attempting to keep the 
simple-switch architecture, with the addition of some functionality 
added to externs.</div><div><br></div><div>Thank you for your time,</div><div>Isaac</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 27, 2018 at 4:20 AM Antonin Bas <<a href="mailto:antonin@barefootnetworks.com" target="_blank">antonin@barefootnetworks.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Isaac,<div><br></div><div>Yes, please try to submit your features as separate pull requests.</div><div>Also before submitting a patch, please ask yourself these questions:</div><div>- does it belong in the core library (bm_sim) and can be used by several / most architectures, or is it architecture specific?</div><div>- does it slow down packet processing for P4 programs that do not utilize the new feature?</div><div>- if adding a feature to simple_switch, it is something that is required by the P4_16 v1model architecture - or at least compatible with it?</div><div><br></div><div>For example, 3) (timer) seems like a good candidate for the bmv2 core library, providing that it is implemented in such a way that there is no penalty for programs that do not require it.</div><div><br></div><div>Looking forward to review your PRs.</div><div><br></div><div>Antonin</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Sep 26, 2018 at 11:53 AM Isaac Pedisich <<a href="mailto:iped@seas.upenn.edu" target="_blank">iped@seas.upenn.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Dear All,</div><div><br></div><div>Recently, we have
 made some small modifications (fewer than 200 lines) to the behavioral model's simple switch 
target to introduce a bit of new functionality, centered around easing the execution of arbitrary externs. We'd like to explore the 
options of giving these changes to the community.<br></div><div><br></div><div>There are three pieces of functionality that these modifications implement. In brief, they are:</div><div><br></div><div>1)
 Packet serialization: We implemented a few functions to serialize an entire 
packet (including valid headers, or a manually specified set of headers)
 into a character array. We also implemented deserialization back into 
the packet and headers, so packets can be modified within an extern that expects to operate on packets as byte arrays.</div><div><br></div><div>2)
 Packet creation: We implemented functions to inject arbitrarily 
generated packets into various stages in the pipeline, allowing externs 
to generate entirely new packets.<br></div><div><br></div><div>3)
 Periodic extern-helper execution: This allowed extern functions to 
register a callback and interval to the switch. If the extern is 
declared in the P4 code, the callback is called every time the interval 
elapses.</div><div><br></div><div>We would gladly work with the 
community to make the changes acceptable to you, if you feel that the 
changes would be as helpful to others as they have been to us.</div><div><br></div><div>To facilitate merging, we are happy to submit these features as separate pull requests so they may be accepted independently of one another. <br></div><div><br></div><div>Thank you.</div></div>
_______________________________________________<br>
P4-dev mailing list<br>
<a href="mailto:P4-dev@lists.p4.org" target="_blank">P4-dev@lists.p4.org</a><br>
<a href="http://lists.p4.org/mailman/listinfo/p4-dev_lists.p4.org" rel="noreferrer" target="_blank">http://lists.p4.org/mailman/listinfo/p4-dev_lists.p4.org</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="m_5357458791756490545m_-8509286553622062463m_-7257778801988925969gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Antonin<br></div></div>
</blockquote></div>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="m_5357458791756490545gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Antonin<br></div></div>